-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat implement tutorials mode #63
base: main
Are you sure you want to change the base?
Feat implement tutorials mode #63
Conversation
…sephchimebuka/zkube into feat--implement-tutorial-mode
…sephchimebuka/zkube into feat-implement-tutorials-mode
@josephchimebuka is attempting to deploy a commit to the z-korp Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request primarily focus on implementing a new "Tutorial" mode to enhance the gameplay experience. This includes modifications to various components, such as the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TutorialModal
participant TutorialGrid
participant Game
User->>TutorialModal: Open Tutorial
TutorialModal->>User: Show Tutorial Instructions
User->>TutorialGrid: Start Tutorial
TutorialGrid->>Game: Initialize Game State
Game->>User: Display Game Grid
User->>TutorialGrid: Interact with Blocks
TutorialGrid->>User: Provide Feedback
User->>TutorialGrid: Complete Step
TutorialGrid->>TutorialModal: Step Complete Notification
TutorialModal->>User: Show Next Step Instructions
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 25
🧹 Outside diff range and nitpick comments (45)
client/src/enums/bonusEnum.ts (1)
1-6
: Consider using explicit string values for enum members.While numeric enums work, using string values makes the code more maintainable and safer, especially if these values need to be serialized or used in APIs.
export enum BonusName { - WAVE, - TIKI, - HAMMER, - NONE, + WAVE = 'WAVE', + TIKI = 'TIKI', + HAMMER = 'HAMMER', + NONE = 'NONE', }client/src/ui/utils.ts (1)
Line range hint
1-7
: LGTM! Consider adding documentation.The implementation correctly combines
clsx
andtailwind-merge
for robust class name handling. This will be useful for managing tutorial UI styling.Consider adding JSDoc documentation with an example:
+/** + * Combines multiple class names and merges Tailwind CSS classes efficiently. + * @example + * // Basic usage + * cn('px-2 py-1', 'bg-blue-500', { 'text-white': true, 'rounded': false }) + * // => 'px-2 py-1 bg-blue-500 text-white' + * + * // Merging conflicting Tailwind classes + * cn('px-2 py-1', 'px-4') + * // => 'py-1 px-4' + */ export function cn(...inputs: ClassValue[]) { return twMerge(clsx(inputs)); }client/src/ui/containers/Header.tsx (1)
8-10
: LGTM! Consider adding JSDoc comments.The interface is well-defined and follows TypeScript best practices. Consider adding JSDoc comments to document the purpose and usage of the
onStartTutorial
callback.+/** + * Props for the Header component + */ interface HeaderProps { + /** Callback function to initiate the tutorial mode */ onStartTutorial: () => void; }client/src/hooks/useGame.tsx (2)
Line range hint
8-13
: Remove or implement the unusedshouldLog
parameter.The
shouldLog
parameter is defined in the type definition but not used in the implementation. This creates a TypeScript error as required parameters must be used.Apply this diff to fix the issue:
export const useGame = ({ gameId, -}: { - gameId: string | undefined; - shouldLog: boolean; +}: { + gameId: string | undefined; }) => {If logging is needed for tutorial mode, please implement the logging functionality instead of removing the parameter.
Based on the code analysis, I can now provide the final response:
Hook implementation needs tutorial-specific tracking capabilities
The current implementation of
useGame
hook lacks tutorial-specific features that are needed by the tutorial implementation:
- The tutorial components track game state, progress, and steps but can't access these through the hook
- Tutorial-specific logging for progression tracking is done directly in components
- The hook should provide tutorial state management capabilities
Required changes:
- Add tutorial mode detection and state tracking
- Include tutorial-specific game state validation
- Expose tutorial progression methods
🔗 Analysis chain
Line range hint
15-34
: LGTM! Core implementation looks solid.The hook follows React best practices:
- Proper use of
useMemo
for memoization- Clean separation of concerns
- Efficient game entity retrieval
Let's verify if this hook meets the tutorial mode requirements:
Let me gather more information about the tutorial implementation to ensure the hook meets all requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if this hook is used in tutorial-related components # and if logging might be needed for tutorial progression tracking # Search for tutorial-related files echo "Searching for tutorial-related files..." fd -e tsx -e ts tutorial # Check usage of useGame hook in tutorial components echo "Checking useGame usage in tutorial components..." rg -l "useGame" | rg -i "tutorial" # Look for any logging patterns in tutorial components echo "Checking existing logging patterns..." rg "console\.|log\(" | rg -i "tutorial"Length of output: 1651
Script:
#!/bin/bash # Check the tutorial components implementation to understand game state handling echo "Checking TutorialComponent implementation..." cat client/src/ui/modules/TutorialComponent.tsx # Check Tutorial screen implementation echo "Checking Tutorial screen implementation..." cat client/src/ui/screens/Tutorial.tsx # Look for any game state tracking or progression patterns echo "Looking for game state patterns in tutorial..." rg -A 5 "gameState|progress|step" client/src/ui/components/TutorialLength of output: 15373
client/src/hooks/useAccountCustom.tsx (2)
9-9
: Consider making ACCOUNT_CONNECTOR constant and adding documentation.While the change to use "burner" as default aligns well with the tutorial mode implementation, consider:
- Using
const
instead oflet
since this appears to be a configuration value- Adding JSDoc comments to explain the purpose and impact of this setting
- export let ACCOUNT_CONNECTOR: AccountType = "burner"; + /** + * Determines the default account type for the application. + * - "burner": Used for tutorial mode and off-chain interactions + * - "controller": Used for on-chain wallet connections + */ + export const ACCOUNT_CONNECTOR: AccountType = "burner";
Line range hint
9-35
: LGTM! Architectural decision aligns with tutorial mode requirements.The switch to using burner accounts as default is a good architectural decision because:
- It enables off-chain tutorial mode without requiring wallet connection
- Provides a frictionless onboarding experience for new users
- The implementation cleanly handles both burner and controller account types
- Proper error handling is in place for null cases
The code maintains flexibility to switch back to controller accounts when needed.
client/src/dojo/game/types/mode.ts (3)
15-15
: Use Title Case for consistency with other mode names.The mode name "tutorial" should follow the same casing convention as other modes.
- Tutorial = "tutorial" + Tutorial = "Tutorial Mode"
38-39
: Consider grouping zero-duration modes together.For better maintainability, consider grouping the Tutorial case with the None case since both return 0.
switch (this.value) { case ModeType.None: + case ModeType.Tutorial: return 0; - case ModeType.Tutorial: - return 0; case ModeType.Normal:
63-67
: Remove extra blank lines and add JSDoc documentation.The method implementation is correct, but could benefit from documentation and proper formatting.
- - - public isTutorial(): boolean { - return this.value === ModeType.Tutorial; - } + /** + * Checks if the current mode is Tutorial mode + * @returns {boolean} True if the mode is Tutorial, false otherwise + */ + public isTutorial(): boolean { + return this.value === ModeType.Tutorial; + }client/src/ui/components/TutorialModal.tsx (3)
6-10
: Add JSDoc documentation for the interface and props.While the props are well-typed, adding JSDoc documentation would improve maintainability and developer experience.
+/** + * Props for the TutorialModal component + */ interface TutorialModalProps { + /** Controls the visibility of the modal */ isOpen: boolean; + /** Callback function when the modal is closed */ onClose: () => void; + /** Callback function to initiate the tutorial */ onStartTutorial: () => void; }
11-17
: Consider memoizing image assets.The image assets are recalculated on every render. Consider using useMemo to optimize performance.
const TutorialModal = ({ isOpen, onClose, onStartTutorial, }: TutorialModalProps) => { const { themeTemplate } = useTheme(); - const imgAssets = ImageAssets(themeTemplate); + const imgAssets = React.useMemo(() => ImageAssets(themeTemplate), [themeTemplate]);
48-48
: Consider using named export instead of default export.Named exports are generally preferred for better tree-shaking and explicit imports.
-export default TutorialModal; +export { TutorialModal };client/src/ui/containers/GameBonus.tsx (1)
4-5
: Remove unused BonusType import.Since the component has been refactored to use
BonusName
instead ofBonusType
, theBonusType
import is no longer needed.-import { BonusType } from "@/dojo/game/types/bonus"; import { BonusName } from "@/enums/bonusEnum";
client/src/ui/components/BonusButton.tsx (1)
17-19
: Consider consolidating redundant propsThe
bonusName
andbonus
props appear to serve similar purposes. Consider consolidating them into a single prop for better maintainability.interface BonusButtonProps { onClick?: () => void; urlImage: string; bonusCount: number; - tooltipText: string; // Ajout d'une nouvelle prop pour le texte du tooltip - bonusName: BonusName; - bonus: BonusName; + tooltipText: string; // Added new prop for tooltip text + selectedBonus: BonusName; }Update the component usage accordingly:
- bonus == bonusName + selectedBonus === currentBonus // assuming currentBonus is the active bonusclient/src/ui/modules/TutorialComponent.tsx (1)
1-9
: Remove unused import and add documentation.The
Game
import appears to be unused. Additionally, the interface would benefit from JSDoc documentation for better maintainability.-import { Game } from '@/dojo/game/models/game'; import { Block } from '@/dojo/game/models/TutorialGrid'; import { CubeSlidingTutorial } from '@/dojo/game/models/CubeSlidingTutorial'; +/** + * Props for the TutorialComponent + * @param tutorial - The tutorial instance managing the game state and progression + * @param onEndTutorial - Callback function triggered when the tutorial is completed + */ interface TutorialComponentProps { tutorial: CubeSlidingTutorial; onEndTutorial: () => void; }client/src/ui/components/DesktopHeader.tsx (2)
31-46
: Consider memoizing handler functions.The tutorial handler functions could benefit from memoization using
useCallback
to prevent unnecessary rerenders, especially since they're passed as props to child components.- const handleTutorialOpen = () => { + const handleTutorialOpen = useCallback(() => { setIsTutorialOpen(true); - }; + }, []); - const handleTutorialClose = () => { + const handleTutorialClose = useCallback(() => { setIsTutorialOpen(false); - }; + }, []); - const handleStartTutorial = () => { + const handleStartTutorial = useCallback(() => { handleTutorialClose(); onStartTutorial(); - }; + }, [handleTutorialClose, onStartTutorial]);
68-77
: Add accessibility attributes to the Tutorial button.The Tutorial button should include ARIA attributes to improve accessibility. Also, consider adding a tooltip to provide more context about the tutorial feature.
<Button variant="outline" + aria-label="Open tutorial mode" + title="Learn how to play the game" onClick={(e) => { e.stopPropagation(); handleTutorialOpen(); }} > Tutorial </Button>client/src/dojo/game/models/game.ts (3)
Line range hint
65-65
: Avoid hard-coding the buyIn value.The buyIn value is hard-coded to 100 without explanation. Consider:
- Making it configurable based on game mode
- Setting it to 0 for tutorial mode
- Moving it to constants file
- this.buyIn = 100; // Set default buy-in of $100 + this.buyIn = this.mode.isTutorial() ? 0 : DEFAULT_GAME_BUYIN;
108-109
: Remove unnecessary empty lines.These consecutive empty lines don't serve any purpose and should be removed.
Line range hint
111-122
: Update utility methods to handle Tutorial mode.The utility methods need to account for the new Tutorial mode:
isOver()
might need different logic for tutorial progressionisPaid()
should always return false for tutorialsgetGameModeMultiplier()
should handle tutorial scoring differentlypublic isPaid(): boolean { + if (this.mode.isTutorial()) { + return false; + } return this.mode.price() > 0n; } public getGameModeMultiplier(): number { + if (this.mode.isTutorial()) { + return GAME_MODE_TUTORIAL_MULTIPLIER; + } return this.mode.price() === 0n ? GAME_MODE_FREE_MULTIPLIER : GAME_MODE_PAID_MULTIPLIER; }client/src/ui/components/TutorialBlock.tsx (4)
5-24
: Add JSDoc documentation for the interface and its properties.While the interface is well-structured, adding JSDoc documentation would improve maintainability and help other developers understand the purpose of each prop.
+/** + * Props for the BlockContainer component used in tutorial mode + */ interface BlockProps { + /** The block data to render */ block: Block; + /** Size of the grid cell in pixels */ gridSize: number; + /** Whether a transaction is being processed */ isTxProcessing?: boolean; // ... add documentation for other props }
43-63
: Remove console.log statements from production code.Consider replacing console.log statements with proper logging or removing them entirely for production code.
const onTransitionStart = () => { - console.log("Transition started for block", block); onTransitionBlockStart(); setTransition("Start"); }; const handleTransitionEnd = () => { - console.log("Transition ended for block", block); setTransition("End"); onTransitionBlockEnd(); };
75-105
: Consider extracting positioning and transition logic.The positioning and transition logic could be extracted into separate utility functions or custom hooks for better maintainability and reusability.
+const getBlockStyle = (block: Block, gridSize: number, state: GameState, transitionDuration: number) => ({ + position: "absolute" as const, + top: `${block.y * gridSize + 1}px`, + left: `${block.x * gridSize + 1}px`, + width: `${block.width * gridSize}px`, + height: `${gridSize}px`, + transition: [GameState.GRAVITY, GameState.GRAVITY2, GameState.GRAVITY_BONUS].includes(state) + ? `top ${transitionDuration / 1000}s linear` + : "none", + color: "white", +}); return ( <div className={`block block-${block.width} ${isTxProcessing ? "cursor-wait" : ""} ${highlightClass} ${isBlockClickable ? 'cursor-pointer' : ''}`} ref={ref} - style={{ - position: "absolute", - top: `${block.y * gridSize + 1}px`, - left: `${block.x * gridSize + 1}px`, - width: `${block.width * gridSize}px`, - height: `${gridSize}px`, - transition: - state === GameState.GRAVITY || - state === GameState.GRAVITY2 || - state === GameState.GRAVITY_BONUS - ? `top ${transitionDuration / 1000}s linear` - : "none", - color: "white", - }} + style={getBlockStyle(block, gridSize, state, transitionDuration)}
26-106
: Consider memoizing the component for performance.Since the component might be rendered multiple times in a grid, consider using React.memo to prevent unnecessary re-renders when props haven't changed.
-const BlockContainer: React.FC<BlockProps> = ({ +const BlockContainer: React.FC<BlockProps> = React.memo(({ // ... props -}); +}));client/src/index.css (3)
145-155
: Well-implemented highlight class with accessibility considerations!The
.highlight
class implementation is well-designed with:
- Appropriate visual emphasis using gold border and glow effect
- Smooth transitions for better user experience
- Proper accessibility support via
prefers-reduced-motion
Consider documenting this class in your UI component library or style guide to promote reuse across tutorial-related components.
Line range hint
156-183
: Remove duplicate animation classes.The
.wiggle
,.blink
, and.wiggle-blink
classes are duplicated in the file. This could lead to confusion and maintenance issues.Remove one set of these duplicate classes:
.wiggle { animation: wiggle 0.2s ease-in-out infinite; } .blink { animation: blink 0.5s ease-in-out infinite; } .wiggle-blink { animation: wiggle 0.2s ease-in-out infinite, blink 0.5s ease-in-out infinite; } - .wiggle { - animation: wiggle 0.2s ease-in-out infinite; - } - - .blink { - animation: blink 0.5s ease-in-out infinite; - } - - .wiggle-blink { - animation: - wiggle 0.2s ease-in-out infinite, - blink 0.5s ease-in-out infinite; - }
Line range hint
113-144
: Add reduced motion support for animations.The animation classes should respect user preferences for reduced motion, similar to the
.highlight
class implementation.Add media queries for reduced motion:
@keyframes wiggle { 0%, 100% { transform: translateX(0); } 50% { transform: translateX(5px); } } @keyframes blink { 0%, 100% { opacity: 1; } 50% { opacity: 0.5; } } .wiggle { animation: wiggle 0.2s ease-in-out infinite; + @media (prefers-reduced-motion: reduce) { + animation: none; + } } .blink { animation: blink 0.5s ease-in-out infinite; + @media (prefers-reduced-motion: reduce) { + animation: none; + } } .wiggle-blink { animation: wiggle 0.2s ease-in-out infinite, blink 0.5s ease-in-out infinite; + @media (prefers-reduced-motion: reduce) { + animation: none; + } }client/src/ui/actions/Start.tsx (1)
Line range hint
30-149
: Consider tutorial mode integration in handleClick.Given the PR's objective to implement a tutorial mode, the
handleClick
function should potentially handle tutorial mode differently from normal/daily modes (e.g., skip credit checks, use predetermined seeds).Consider adding a conditional branch for tutorial mode:
const handleClick = useCallback(async () => { if (mode === ModeType.Tutorial) { // Tutorial-specific logic handleGameMode(); return; } // Existing logic for normal/daily modes ... }, [/* existing dependencies */]);client/src/ui/components/TreasureChest.tsx (1)
Line range hint
22-171
: Consider tutorial mode integration.Since this PR implements a tutorial mode, consider how this component might need to adapt for tutorial scenarios. For example:
- Should certain features be disabled during tutorials?
- Will tutorial mode need special handling of chest progression?
- Should the UI display additional guidance during tutorials?
client/src/ui/components/GameBoard.tsx (3)
Line range hint
31-57
: Consider enhancing GameBoardProps for tutorial mode supportThe component currently lacks tutorial mode specific properties. Consider adding tutorial-related props to support guided gameplay features.
interface GameBoardProps { initialGrid: number[][]; nextLine: number[]; score: number; combo: number; maxCombo: number; hammerCount: number; waveCount: number; totemCount: number; account: Account | null; game: Game; + isTutorialMode?: boolean; + currentTutorialStep?: number; + tutorialHighlight?: { + element: string; + message: string; + }; }
Line range hint
73-106
: Enhance bonus handlers with tutorial-specific guidanceThe bonus click handlers could be enhanced to provide additional guidance and feedback during tutorial mode.
Example enhancement for the Wave bonus:
const handleBonusWaveClick = () => { if (waveCount === 0) return; if (bonus === BonusType.Wave) { setBonus(BonusType.None); setBonusDescription(""); } else { setBonus(BonusType.Wave); - setBonusDescription("Select the line you want to destroy"); + setBonusDescription( + props.isTutorialMode + ? "Tutorial: The Wave bonus destroys an entire row! Select any block in the row you want to clear." + : "Select the line you want to destroy" + ); } };
Line range hint
219-308
: Add tutorial overlay and guidance systemThe render section should include tutorial-specific UI elements to guide players through the game mechanics.
Consider adding:
- A tutorial overlay component for step-by-step instructions
- Visual highlights for interactive elements
- Progress indicators for tutorial steps
- Conditional rendering of tournament info based on game mode
Example implementation:
// Add before the return statement const renderTutorialOverlay = () => { if (!props.isTutorialMode) return null; return ( <TutorialOverlay step={props.currentTutorialStep} highlightElement={props.tutorialHighlight?.element} message={props.tutorialHighlight?.message} /> ); }; // Add within the Card component {renderTutorialOverlay()}client/src/ui/screens/Home.tsx (3)
118-125
: Consolidate tutorial initialization logic.
handleStartTutorial
andstartTutorial
have overlapping responsibilities, which could lead to maintenance issues.Consider consolidating these functions:
-const handleStartTutorial = useCallback(() => { - setTutorialState({ - isActive: true, - showGrid: true, - showText: false, - }); - startTutorial(); -}, [startTutorial]); +const startTutorial = useCallback(() => { + try { + setTutorialState({ + isActive: true, + showGrid: true, + showText: false, + currentStep: 0, + completedSteps: new Set(), + }); + } catch (error) { + handleError(error); + } +}, []);
64-78
: Consider decomposing the Home component to reduce complexity.The component has grown to manage multiple concerns (game state, tutorial state, views, dialogs), making it harder to maintain and test.
Consider:
- Extract tutorial state management into a custom hook:
const useTutorialState = () => { const [tutorialState, setTutorialState] = useState({...}); const startTutorial = useCallback(() => {...}, []); const endTutorial = useCallback(() => {...}, []); return { tutorialState, startTutorial, endTutorial }; };
- Extract view rendering logic into separate components:
const GameModeSelector = ({ onModeSelect }) => {...}; const TournamentView = ({ onBack }) => {...};This would improve maintainability and make the component easier to test.
Line range hint
134-170
: Improve effect organization and cleanup handling.The effects are scattered and some cleanup logic might be missed during unmount.
Consider:
- Grouping related effects together
- Adding cleanup for all side effects
- Using a more declarative approach for state updates
// Game state effects useEffect(() => { if (game?.over) { handleGameOver(); } }, [game?.over]); // Tutorial effects useEffect(() => { if (!tutorialState.isActive) { return; } // Setup tutorial const cleanup = setupTutorial(); return () => { cleanup(); handleTutorialCleanup(); }; }, [tutorialState.isActive]);client/src/ui/components/Grid.tsx (3)
62-85
: Consider removing console logs before production deployment.The state management improvements look good, but there are debugging console logs that should be removed or replaced with proper logging.
- console.log( - `Mouvement effectué : Ligne ${rowIndex}, de ${startColIndex} à ${finalColIndex}`, - );
287-296
: Consider optimizing gravity calculations for better performance.The gravity calculation logic could be optimized in two ways:
- Memoize
calculateFallDistance
results to prevent recalculation for unchanged blocks- Use array methods that don't create new arrays on every update
+ const memoizedCalculateFallDistance = useCallback( + (block: Block, blocks: Block[]) => { let maxFall = gridHeight - block.y - 1; for (let y = block.y + 1; y < gridHeight; y++) { if (isCollision(block.x, y, block.width, blocks, block.id)) { maxFall = y - block.y - 1; break; } } return maxFall; + }, + [gridHeight] + );Also applies to: 314-333
440-482
: Consider extracting styles and animations to separate files.The component contains complex inline styles and animation logic. Consider:
- Moving styles to a separate CSS module
- Creating a reusable animation component
+ // styles/Grid.module.css + .gridBackground { + background-image: linear-gradient(#1E293B 2px, transparent 2px), + linear-gradient(to right, #1E293B 2px, #10172A 2px); + } + + // components/animations/BounceAnimation.tsx + const BounceAnimation: React.FC<{children: React.ReactNode}> = ({children}) => ( + <motion.div + animate={shouldBounce ? { scale: [1, 1.1, 1, 1.1, 1] } : {}} + transition={{ duration: 0.2, ease: "easeInOut" }} + > + {children} + </motion.div> + );client/src/ui/screens/Tutorial.tsx (4)
14-14
: Remove unused import ofset
fromdate-fns
.The imported
set
function fromdate-fns
is not used in the code. Removing unused imports can help optimize the bundle size and improve code clarity.Apply this diff:
-import { set } from "date-fns";
151-152
: Unnecessary rounding of integer grid coordinates.In the
selectBlock
function,block.x
is already an integer representing the grid coordinate. UsingMath.round()
is redundant.Apply this diff to simplify the code:
- const finalPosition = Math.round(block.x); + const finalPosition = block.x;
61-63
: Improve function naming for better clarity.The function
updateValue
updates both the score and the intermission state. Renaming it to something likeupdateScoreAndIntermission
would make its purpose clearer.
61-63
: Ensure consistent score updates across functions.Both
updateValue
andselectBlock
functions update the score. To maintain consistency and prevent potential discrepancies, consider centralizing score updates in a single function or state management solution.client/src/ui/components/TutorialGrid.tsx (2)
269-269
: Removeconsole.log
statements from production codeConsider removing or commenting out
console.log
statements to clean up console output in production environments.Affected lines:
- Line 269
- Line 273
- Lines 293-295
- Lines 322-324
Also applies to: 273-273, 293-295, 322-324
158-158
: Clean up commented-out codeThere are several blocks of commented-out code. Removing unused code improves readability and reduces clutter.
Affected lines:
- Line 158
- Lines 196-197
- Line 226
- Lines 276-278
- Lines 289-290
Also applies to: 196-197, 226-226, 276-278, 289-290
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
client/package-lock.json
is excluded by!**/package-lock.json
client/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (25)
client/src/dojo/game/models/game.ts
(1 hunks)client/src/dojo/game/types/difficulty.ts
(1 hunks)client/src/dojo/game/types/mode.ts
(3 hunks)client/src/enums/bonusEnum.ts
(1 hunks)client/src/hooks/useAccountCustom.tsx
(1 hunks)client/src/hooks/useGame.tsx
(1 hunks)client/src/index.css
(1 hunks)client/src/ui/actions/Start.tsx
(1 hunks)client/src/ui/components/BonusButton.tsx
(1 hunks)client/src/ui/components/ContentTabs.tsx
(1 hunks)client/src/ui/components/DesktopHeader.tsx
(2 hunks)client/src/ui/components/GameBoard.tsx
(2 hunks)client/src/ui/components/Grid.tsx
(13 hunks)client/src/ui/components/TreasureChest.tsx
(1 hunks)client/src/ui/components/TutorialBlock.tsx
(1 hunks)client/src/ui/components/TutorialGrid.tsx
(1 hunks)client/src/ui/components/TutorialModal.tsx
(1 hunks)client/src/ui/containers/GameBonus.tsx
(5 hunks)client/src/ui/containers/Header.tsx
(2 hunks)client/src/ui/modules/TutorialComponent.tsx
(1 hunks)client/src/ui/screens/Home.tsx
(10 hunks)client/src/ui/screens/Loading.tsx
(1 hunks)client/src/ui/screens/Tutorial.tsx
(1 hunks)client/src/ui/utils.ts
(1 hunks)client/src/utils/gridUtils.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- client/src/dojo/game/types/difficulty.ts
- client/src/ui/components/ContentTabs.tsx
- client/src/ui/screens/Loading.tsx
🔇 Additional comments (20)
client/src/ui/containers/Header.tsx (1)
Line range hint 12-26
: Verify: Tutorial accessibility on mobile devices
The tutorial trigger is only passed to DesktopHeader
and not to MobileHeader
. Please confirm if this is an intentional design decision to restrict tutorial access on mobile devices.
If this is intentional, consider:
- Adding a comment explaining this UX decision
- Implementing a mobile-friendly version of the tutorial in the future
- Adding a note in the mobile view about desktop-only tutorial availability
Run this script to check for any existing mobile tutorial implementations:
✅ Verification successful
Based on the search results, I can now provide a definitive response about the tutorial accessibility on mobile devices:
Tutorial is intentionally desktop-only by design
The codebase evidence confirms this is an intentional design decision:
- The
TutorialModal
andTutorialComponent
implementations are specifically tied to desktop interactions - The
MobileHeader
component is intentionally simple with no tutorial-related props or functionality - The tutorial state and controls are only passed to
DesktopHeader
component - The tutorial implementation in
Tutorial.tsx
shows a structured, multi-step flow designed for desktop interaction
Note: While the review comment's suggestions about documenting this decision and considering a mobile version are valid UX recommendations, the current desktop-only implementation appears to be an intentional architectural choice rather than an oversight.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for mobile-related tutorial implementations
# Expected: No results if tutorial is intentionally desktop-only
# Search for tutorial-related code in mobile components
rg -l "tutorial" --glob "**/*[mM]obile*.{tsx,ts,jsx,js}"
# Search for responsive tutorial-related implementations
rg "useMediaQuery.*tutorial" --type ts
Length of output: 97
Script:
#!/bin/bash
# Let's try a broader search to understand the tutorial implementation
# Check all files containing tutorial-related code
rg -l "tutorial" --type ts --type tsx
# Look for the DesktopHeader and MobileHeader components
rg -A 5 "export.*MobileHeader" --type tsx
rg -A 5 "export.*DesktopHeader" --type tsx
# Search for any tutorial-related props or interfaces
rg "interface.*Props.*tutorial|type.*Props.*tutorial" --type ts --type tsx
Length of output: 315
Script:
#!/bin/bash
# Search without specific file type flags since tsx isn't recognized
# Look for tutorial-related code and components
# Find all TypeScript-like files first
fd "\.(ts|tsx)$" | while read -r file; do
echo "=== $file ==="
rg "tutorial" "$file" || true
done
# Look specifically for Mobile/Desktop header components
fd "\.(ts|tsx)$" | while read -r file; do
echo "=== $file ==="
rg "(Mobile|Desktop)Header" "$file" || true
done
# Search for Props interfaces/types
fd "\.(ts|tsx)$" | while read -r file; do
echo "=== $file ==="
rg "interface.*Props|type.*Props" "$file" || true
done
Length of output: 100993
client/src/dojo/game/types/mode.ts (1)
Line range hint 15-67
: Implementation aligns well with Tutorial mode requirements.
The changes provide a solid foundation for the Tutorial mode implementation, correctly handling duration and pricing while maintaining separation from the main game modes. This aligns with the PR objective of implementing an off-chain tutorial system.
client/src/ui/components/TutorialModal.tsx (1)
1-48
: Verify proper integration with parent components.
Let's verify that all parent components correctly pass the required props to TutorialModal.
✅ Verification successful
Let me try a different approach to verify the integration.
Let me check the implementation of the tutorial-related functionality in the parent components.
Based on the verification results, I can now provide the final response:
TutorialModal integration is properly implemented
The component is correctly integrated in DesktopHeader with all required props:
isOpen
is controlled viaisTutorialOpen
stateonClose
is handled byhandleTutorialClose
onStartTutorial
is properly chained throughhandleStartTutorial
which closes the modal and calls the parent'sonStartTutorial
The implementation follows React best practices with proper state management and event handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify TutorialModal integration in parent components
# Test: Check proper prop passing in parent components
echo "Checking TutorialModal usage in parent components..."
rg -A 5 "TutorialModal" --type tsx
# Test: Verify onStartTutorial prop implementation
echo "Checking onStartTutorial implementation..."
ast-grep --pattern 'const $name = $$$; const handleStartTutorial = $_'
Length of output: 345
Script:
#!/bin/bash
# Search for TutorialModal usage with ripgrep
echo "Checking TutorialModal usage..."
rg "TutorialModal" -A 10 -B 2
# Search for files containing TutorialModal
echo -e "\nFiles containing TutorialModal:"
rg "TutorialModal" --files-with-matches
# Find all tsx files that might contain the component
echo -e "\nAll potential tsx/ts files:"
fd "\.(tsx|ts)$"
Length of output: 10585
Script:
#!/bin/bash
# Check implementation in DesktopHeader
echo "Checking DesktopHeader implementation..."
rg "handleTutorial|handleStartTutorial|isTutorialOpen" -B 2 -A 5 client/src/ui/components/DesktopHeader.tsx
# Check Tutorial screen implementation
echo -e "\nChecking Tutorial screen implementation..."
rg "onStartTutorial|handleStartTutorial" -B 2 -A 5 client/src/ui/screens/Tutorial.tsx
# Check TutorialComponent implementation
echo -e "\nChecking TutorialComponent implementation..."
rg "onStartTutorial|handleStartTutorial" -B 2 -A 5 client/src/ui/modules/TutorialComponent.tsx
Length of output: 1616
client/src/ui/containers/GameBonus.tsx (2)
37-38
: LGTM! BonusButton updates are consistent and well-documented.
The changes to use BonusName
enum values are consistent across all bonus buttons. The tooltips provide clear descriptions that will be helpful for the tutorial mode implementation.
Also applies to: 47-48, 57-58
14-14
: Verify GameBonus component usage with new BonusName type.
The type change from BonusType
to BonusName
looks good, but we should verify all component usages have been updated.
✅ Verification successful
The search results don't show the actual bonus
prop being passed to the GameBonus
component. Let me search for more specific information about the type usage.
Type change from BonusType to BonusName is correctly implemented across the codebase
The verification shows that:
BonusName
enum is properly defined inenums/bonusEnum.ts
with all required values (WAVE, TIKI, HAMMER, NONE)- All usages of
GameBonus
component inGameBoard.tsx
andTutorial.tsx
are consistent with the type change - The
bonus
prop is correctly typed asBonusName
in theGameBonusProps
interface - All bonus-related logic in the codebase (Grid, TutorialGrid, BonusButton components) uses the
BonusName
type consistently
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of GameBonus component to verify they're using BonusName
# Expected: All instances should use BonusName values for the bonus prop
# Search for GameBonus component usage
rg -l "GameBonus" | xargs rg -A 5 "<GameBonus"
Length of output: 1430
Script:
#!/bin/bash
# Search for the BonusName type definition and its values
rg "BonusName" -A 5
# Search for GameBonusProps interface
ast-grep --pattern 'interface GameBonusProps {
$$$
}'
Length of output: 11716
client/src/ui/components/BonusButton.tsx (1)
Line range hint 21-85
: Implementation looks good!
The component implementation effectively combines:
- Interactive animations for user engagement
- Clear visual feedback for selected bonuses
- Informative tooltips for user guidance
These features align well with the tutorial mode requirements.
client/src/ui/modules/TutorialComponent.tsx (1)
1-60
: Verify tutorial implementation against requirements.
Please ensure that this implementation aligns with the PR objectives:
- The component should support predetermined grid layouts for each tutorial step
- Visual cues and feedback mechanisms should be clearly implemented
- The progression system should prevent advancing until each step is completed
✅ Verification successful
Based on the gathered information, I can now provide the final response:
Tutorial implementation meets all requirements with robust progression system and clear visual feedback.
The implementation successfully addresses all requirements:
- ✓ Predetermined grid layouts: Tutorial manages grid state through
getGridState()
with specific block positions for each step - ✓ Visual feedback mechanisms:
- Highlighted blocks/rows using ring effects and animations
- Clear step instructions displayed for each tutorial phase
- Interactive elements only enabled for relevant tutorial targets
- ✓ Structured progression system:
- 5 well-defined tutorial steps with specific objectives
- Step completion validation before progression
- Prevents advancing until current step is completed
- Clear visual indicators for clickable elements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for related tutorial implementation files
echo "Checking for related tutorial implementation files..."
fd -e tsx -e ts tutorial
# Look for grid layout definitions
echo "\nChecking for grid layout definitions..."
rg -i "tutorial.*grid.*layout" -A 5
# Check for visual feedback implementations
echo "\nChecking for visual feedback mechanisms..."
rg -i "tutorial.*(feedback|cue|hint)" -A 5
Length of output: 625
Script:
#!/bin/bash
# Let's examine the tutorial-related files more closely
# Check TutorialGrid implementation
echo "=== TutorialGrid.tsx content ==="
cat client/src/ui/components/TutorialGrid.tsx
# Check CubeSlidingTutorial model
echo -e "\n=== Looking for CubeSlidingTutorial implementation ==="
fd -e ts CubeSlidingTutorial
# If found, examine its content
if [ -f "client/src/dojo/game/models/CubeSlidingTutorial.ts" ]; then
echo -e "\n=== CubeSlidingTutorial.ts content ==="
cat client/src/dojo/game/models/CubeSlidingTutorial.ts
fi
# Check for tutorial step definitions
echo -e "\n=== Looking for tutorial steps/instructions ==="
rg -i "class.*tutorial" -A 10
Length of output: 19528
Script:
#!/bin/bash
# Let's check for tutorial step and progression implementation
# Look for tutorial step definitions and progression logic
echo "=== Checking tutorial step progression ==="
rg -i "tutorialStep|isTutorialComplete" -A 5
# Check for visual feedback in TutorialBlock component
echo -e "\n=== TutorialBlock.tsx content ==="
cat client/src/ui/components/TutorialBlock.tsx
# Look for grid layout initialization
echo -e "\n=== Checking grid initialization ==="
rg -i "getGridState|initialData.*tutorial" -A 5
Length of output: 15913
client/src/ui/components/DesktopHeader.tsx (2)
14-14
: LGTM! Clean interface definition and import.
The DesktopHeaderProps
interface is well-defined and the prop name clearly indicates its purpose.
Also applies to: 18-20
Line range hint 1-99
: Verify tutorial mode integration across components.
The implementation provides a good entry point for the tutorial mode. Let's verify the integration with other components.
✅ Verification successful
Tutorial mode implementation is well integrated across components
The verification shows a complete tutorial mode implementation with proper component hierarchy and state management:
- Entry point in
DesktopHeader.tsx
properly connects toTutorialModal
component - Tutorial flow is managed through:
Home.tsx
: Main tutorial state managementTutorial.tsx
: Step-by-step tutorial flowTutorialComponent.tsx
: Core tutorial logic and grid stateTutorialGrid.tsx
: Visual tutorial elements
- Props and state are correctly passed down through components (
onStartTutorial
,tutorialState
, etc.) - Integration includes both UI components and game logic (via imports from
@/dojo/game/models/
)
The implementation aligns with PR objectives for tutorial mode framework and flow control.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify tutorial mode integration across components
# Test 1: Check for tutorial-related components and their connections
echo "Checking for tutorial-related components..."
rg -l "Tutorial" --type tsx
# Test 2: Verify onStartTutorial prop usage
echo "Checking onStartTutorial prop usage..."
rg "onStartTutorial" --type tsx
# Test 3: Look for tutorial state management
echo "Checking tutorial state management..."
rg "tutorial.*State|useTutorial" --type tsx
Length of output: 458
Script:
#!/bin/bash
# Description: Verify tutorial mode integration across components
# Test 1: Check for tutorial-related components and their connections
echo "Checking for tutorial-related components..."
rg -l "Tutorial"
# Test 2: Verify onStartTutorial prop usage
echo "Checking onStartTutorial prop usage..."
rg "onStartTutorial" -A 2
# Test 3: Look for tutorial state management
echo "Checking tutorial state management..."
rg "tutorial.*State|useTutorial"
# Test 4: Check for tutorial-related imports and exports
echo "Checking tutorial-related imports and exports..."
rg "import.*Tutorial|export.*Tutorial"
Length of output: 4956
client/src/dojo/game/models/game.ts (3)
Line range hint 74-89
: LGTM: Block unpacking implementation is robust.
The block unpacking logic correctly:
- Handles the bitmap conversion
- Preserves grid dimensions
- Maintains block ordering through reverse()
Line range hint 1-122
: Verify Tutorial mode enum integration.
Ensure that the Mode enum properly includes and handles the Tutorial mode type.
#!/bin/bash
# Check Mode enum implementation
rg -t typescript "enum.*Mode|type.*Mode" -A 5
Line range hint 19-45
: Consider adding tutorial-specific properties to support the new Tutorial mode.
Given that this PR implements a Tutorial mode, the Game class should include properties to track tutorial state and progress. Consider adding:
tutorialStep: number
tutorialCompleted: boolean
tutorialConfig: TutorialConfig
Let's verify if tutorial-related types exist:
client/src/ui/components/TutorialBlock.tsx (3)
26-39
: LGTM! Well-structured component declaration.
The component declaration follows React best practices with proper TypeScript typing and sensible default values for optional props.
65-70
: LGTM! Clean and maintainable styling logic.
The conditional styling with Tailwind CSS classes is well-implemented, with clear visual distinction between row and block highlights.
72-73
: LGTM! Well-implemented click handling for tutorial mode.
The conditional click handling aligns perfectly with the tutorial mode requirements, ensuring users can only interact with highlighted elements.
client/src/ui/components/TreasureChest.tsx (2)
Line range hint 22-26
: LGTM! Well-structured component declaration.
The component follows React and TypeScript best practices with proper typing and clear prop definitions.
Line range hint 28-29
: Remove commented-out code.
The component contains commented-out state variables and UI elements related to leaderboard functionality. If this functionality is planned for future implementation, it should be tracked in an issue rather than kept as commented code.
Apply this diff to clean up the code:
- // const [showLeaderboard, setShowLeaderboard] = useState(false);
- // const [selectedChest, setSelectedChest] = useState<Chest | null>(null);
- {/* Leaderboard Toggle */}
- {/*<button
- className="mt-4 px-4 py-2 bg-blue-500 text-white rounded hover:bg-blue-600 transition-colors"
- onClick={() => setShowLeaderboard(!showLeaderboard)}
- >
- {showLeaderboard ? "Hide Leaderboard" : "Show Leaderboard"}
- </button>*/}
-
- {/* Leaderboard Panel */}
Let's check if there are any other related leaderboard references in the codebase:
Also applies to: 147-157
client/src/utils/gridUtils.ts (1)
102-103
: LGTM!
The added empty lines improve code readability by providing better separation between function blocks.
client/src/ui/components/Grid.tsx (2)
Line range hint 1-29
: LGTM! Props interface update aligns with tutorial mode requirements.
The change from BonusType
to BonusName
in the GridProps
interface improves type safety and better reflects the bonus system's role in the tutorial mode.
349-402
: Verify state transition safety in concurrent scenarios.
The state machine logic handles multiple states, but there could be race conditions when rapid state transitions occur. Consider implementing a state transition queue or debounce mechanism.
client/src/ui/screens/Tutorial.tsx
Outdated
[3, 0, 0, 2, 0, 0, 0, 0], | ||
[0, 1, 2, 3, 0], | ||
[2, 0, 0, 3, 2], | ||
[1, 0, 2, 2, 0, 0, 1, 1], | ||
[3, 0, 0, 1, 0, 0, 2], // 9th row |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent gridData
row lengths may cause rendering issues.
The gridData
array has rows with varying lengths, which could lead to unexpected behavior when rendering the grid. For instance, rows at lines 44 and 45 have only 5 elements, while others have 7 or 8. Ensure all rows have the same number of columns to maintain a consistent grid structure.
Apply this diff to standardize the row lengths:
[3, 0, 0, 2, 0, 0, 0, 0],
- [0, 1, 2, 3, 0],
- [2, 0, 0, 3, 2],
+ [0, 1, 2, 3, 0, 0, 0, 0],
+ [2, 0, 0, 3, 2, 0, 0, 0],
[1, 0, 2, 2, 0, 0, 1, 1],
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
[3, 0, 0, 2, 0, 0, 0, 0], | |
[0, 1, 2, 3, 0], | |
[2, 0, 0, 3, 2], | |
[1, 0, 2, 2, 0, 0, 1, 1], | |
[3, 0, 0, 1, 0, 0, 2], // 9th row | |
[3, 0, 0, 2, 0, 0, 0, 0], | |
[0, 1, 2, 3, 0, 0, 0, 0], | |
[2, 0, 0, 3, 2, 0, 0, 0], | |
[1, 0, 2, 2, 0, 0, 1, 1], | |
[3, 0, 0, 1, 0, 0, 2], // 9th row |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (5)
client/src/ui/screens/Tutorial.tsx (1)
35-35
: Add proper typing for bonus state.The bonus state should leverage the imported
BonusType
enum for better type safety.- const [bonus, setBonus] = useState<BonusType>(BonusType.None); + const [bonus, setBonus] = useState<BonusType | null>(BonusType.None);client/src/ui/screens/Home.tsx (1)
267-267
: Add tutorial availability check to Header.Consider passing additional props to Header to control tutorial button visibility based on user progress.
-<Header onStartTutorial={handleStartTutorial} /> +<Header + onStartTutorial={handleStartTutorial} + showTutorial={!game || game.over} + tutorialCompleted={tutorialState.progression.completed} +/>client/src/ui/components/TutorialGrid.tsx (3)
21-36
: Consider adding JSDoc comments to document theGridProps
interface.While the prop names are self-explanatory, adding JSDoc comments to describe each prop can enhance the interface's readability and maintainability, especially for complex or non-obvious props.
75-77
: Consider moving constants outside the component.The
borderSize
,gravitySpeed
, andtransitionDuration
constants can be moved outside the component since they don't depend on any props or state. This can improve performance by avoiding unnecessary re-creation on each render.
181-202
: Remove commented-out code.The commented-out code in the
handleMouseDown
function should be removed to improve code readability and maintainability. If the code is no longer needed, it's best to remove it entirely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
client/src/dojo/game/models/tournament.ts
(1 hunks)client/src/dojo/game/types/mode.ts
(3 hunks)client/src/ui/components/TutorialGrid.tsx
(1 hunks)client/src/ui/components/TutorialModal.tsx
(1 hunks)client/src/ui/screens/Home.tsx
(4 hunks)client/src/ui/screens/Tutorial.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- client/src/dojo/game/models/tournament.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- client/src/dojo/game/types/mode.ts
- client/src/ui/components/TutorialModal.tsx
🔇 Additional comments (32)
client/src/ui/components/TutorialGrid.tsx (32)
1-20
: LGTM!
The imports are well-organized and cover the necessary dependencies for the component.
38-52
: LGTM!
The component is properly defined using React.FC
and forwardRef
, and the props are correctly destructured.
53-73
: LGTM!
The state variables are appropriately initialized using the useState
hook, and their naming is clear and descriptive.
79-82
: Verify the usage of onUpdate
prop.
The updateValue
function increments the tutorialStep
by 1 and calls the onUpdate
prop with true
. However, it's unclear why true
is always passed. Ensure that this aligns with the expected behavior of the onUpdate
prop.
84-94
: LGTM!
The useEffect
hook is used appropriately to update the component state when the initialData
prop changes. The logic for setting the inDanger
state and updating the lineExplodedCount
is clear and concise.
96-98
: LGTM!
The resetAnimateText
function is a simple and straightforward way to reset the animateText
state to ComboMessages.None
.
100-105
: LGTM!
The useEffect
hook is used effectively to trigger the bounce animation when lineExplodedCount
changes. The timeout is cleared after 0.5 seconds to stop the bouncing.
107-119
: LGTM!
The handleTransitionBlockStart
and handleTransitionBlockEnd
functions correctly update the transitioningBlocks
state by adding or removing block IDs.
121-154
: LGTM!
The handleDragMove
function properly updates the block position based on the drag movement. It checks for boundaries and collisions using the isBlocked
function and updates the initialX
state when necessary.
156-162
: LGTM!
The handleDragStart
function correctly sets the necessary state variables when a drag operation starts.
164-175
: LGTM!
The useBonus
function applies the appropriate bonus action based on the bonus
prop. It updates the blocks
state accordingly and returns a boolean indicating whether a bonus was applied.
177-179
: LGTM!
The useImperativeHandle
hook is used correctly to expose the useBonus
function to the parent component via the ref
prop.
204-207
: LGTM!
The handleTouchStart
function correctly calls the handleDragStart
function with the appropriate arguments.
209-211
: LGTM!
The handleMouseMove
function correctly calls the handleDragMove
function with the appropriate arguments.
213-216
: LGTM!
The handleTouchMove
function correctly calls the handleDragMove
function with the appropriate arguments.
245-247
: LGTM!
The handleTouchEnd
function correctly calls the endDrag
function.
249-261
: LGTM!
The useEffect
hook is used appropriately to add and remove the mouseup
event listener on the document. This ensures that the endDrag
function is called when the mouse is released, even if it's outside the component.
263-301
: LGTM!
The move
function correctly updates the tutorial step based on the start_index
and tutorialStep
values. It also calculates the score based on the move and checks for complete rows.
303-330
: LGTM!
The handleMoveTX
function is memoized using the useCallback
hook and correctly updates the block positions based on the move transaction.
332-359
: LGTM!
The isBlocked
function correctly checks for collisions between blocks during a move operation.
361-370
: LGTM!
The calculateFallDistance
function correctly calculates the maximum fall distance for a block based on collisions with other blocks.
372-386
: LGTM!
The isCollision
function correctly checks for collisions between blocks at a given position.
388-407
: LGTM!
The applyGravity
function correctly updates the block positions based on the calculated fall distances. It also sets the isMoving
state based on whether any blocks have changed position.
409-421
: LGTM!
The useEffect
hook is used appropriately to set up an interval that applies gravity based on the current game state. The interval is cleared when the component unmounts.
423-431
: LGTM!
The handleGravityState
function correctly transitions the game state based on the current state, isMoving
flag, and transitioningBlocks
array.
433-437
: LGTM!
The useEffect
hook is used appropriately to handle gravity state transitions based on the current game state, isMoving
flag, and transitioningBlocks
array.
439-459
: LGTM!
The handleLineClear
function correctly removes complete rows, updates the lineExplodedCount
, and transitions the game state based on the presence of complete rows.
461-477
: LGTM!
The useEffect
hook is used appropriately to handle line clear state transitions based on the current game state and blocks
array.
479-495
: LGTM!
The useEffect
hook is used appropriately to handle the addition of a new line when the game state is GameState.ADD_LINE
and there is a pending move. It correctly concatenates and shifts the blocks, checks if the grid is full, and updates the game state accordingly.
497-511
: LGTM!
The useEffect
hook is used appropriately to handle bonus transactions and move transactions based on the current game state and pending move.
513-525
: LGTM!
The isHighlighted
function correctly determines whether a block should be highlighted based on the current tutorial step and target block.
527-583
: LGTM!
The component's JSX is well-structured and properly uses the motion.div
component from the framer-motion
library for animations. The grid is rendered correctly based on the blocks
array and the BlockContainer
component is used for each block. The AnimatedText
component is used to display combo messages.
{tutorialStep === 1 && ( | ||
<div className="text-center p-4 bg-blue-600 rounded-md mb-4 absolute -left-[22%] mt-2 z-50 w-[155px]"> | ||
<h2> | ||
Step 1: Move the highlighted block two steps to the right. | ||
</h2> | ||
</div> | ||
)} | ||
{tutorialStep === 2 && ( | ||
<div className="text-center p-4 bg-blue-600 rounded-md mb-4 absolute -left-[37%] mt-2 z-50 w-[225px]"> | ||
<h2> | ||
Step 2: Click on the hammer icon on the top left of the game | ||
screen, then click on the highlighted block to use the bonus | ||
and remove it. | ||
</h2> | ||
</div> | ||
)} | ||
{tutorialStep === 3 && ( | ||
<div className="text-center p-4 bg-blue-600 rounded-md mb-4 absolute -left-[37%] mt-2 z-50 w-[225px]"> | ||
<h2> | ||
Step 3: Click on the wave icon on the top left of the game | ||
screen, then double click on the highlighted row of blocks to | ||
use the bonus and remove it. | ||
</h2> | ||
</div> | ||
)} | ||
{tutorialStep === 4 && ( | ||
<div className="text-center p-4 bg-blue-600 rounded-md mb-4 absolute -left-[37%] mt-2 z-50 w-[225px]"> | ||
<h2> | ||
Step 4: Click on the totem icon on the top left of the game | ||
screen, then click on the highlighted block to use the bonus | ||
and remove it and blocks of similar size. | ||
</h2> | ||
</div> | ||
)} | ||
{tutorialStep === 5 && ( | ||
<div className="text-center p-4 bg-blue-600 rounded-md mb-4 absolute left-[14%] top-1/3 mt-2 z-50 w-[355px] h-fit"> | ||
<h2> | ||
Step 5? There's no step 5. You're done! Congratulations! | ||
You've completed the tutorial. use the button below to exit | ||
tutorial mode and play the real game. | ||
</h2> | ||
<button | ||
onClick={endTutorial} | ||
className="mt-4 bg-white text-black px-4 py-3 rounded-md" | ||
> | ||
Exit Tutorial | ||
</button> | ||
</div> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Extract tutorial steps configuration.
The tutorial steps are hardcoded in the JSX, making it difficult to maintain and modify. Consider extracting them to a separate configuration object.
const TUTORIAL_STEPS = {
title: "Step 1",
instruction: "Move the highlighted block two steps to the right.",
position: "-left-[22%]",
width: "w-[155px]"
},
// ... other steps
};
Then simplify the JSX:
- {tutorialStep === 1 && (
- <div className="text-center p-4 bg-blue-600 rounded-md mb-4 absolute -left-[22%] mt-2 z-50 w-[155px]">
- <h2>Step 1: Move the highlighted block two steps to the right.</h2>
- </div>
- )}
+ {TUTORIAL_STEPS[tutorialStep] && (
+ <div className={`text-center p-4 bg-blue-600 rounded-md mb-4 absolute ${TUTORIAL_STEPS[tutorialStep].position} mt-2 z-50 ${TUTORIAL_STEPS[tutorialStep].width}`}>
+ <h2>{TUTORIAL_STEPS[tutorialStep].title}: {TUTORIAL_STEPS[tutorialStep].instruction}</h2>
+ </div>
+ )}
<button | ||
onClick={handleContinue} | ||
className="px-4 py-2 bg-blue-500 text-white rounded-lg" | ||
> | ||
Continue to the Next Step | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add accessibility attributes to interactive elements.
The buttons lack proper accessibility attributes, making them difficult to use with screen readers.
<button
onClick={handleContinue}
+ aria-label="Continue to next tutorial step"
+ role="button"
className="px-4 py-2 bg-blue-500 text-white rounded-lg"
>
Continue to the Next Step
</button>
<button
onClick={endTutorial}
+ aria-label="Exit tutorial mode"
+ role="button"
className="mt-4 bg-white text-black px-4 py-3 rounded-md"
>
Exit Tutorial
</button>
Also applies to: 325-330
const selectBlock = useCallback( | ||
async (block: Block) => { | ||
if (bonus === BonusType.Hammer && tutorialStep === 2) { | ||
const bonusApplied = await applybonus({ | ||
bonus: BonusType.Hammer, | ||
row_index: block.y, | ||
block_index: block.x, | ||
}); | ||
|
||
if (bonusApplied) { | ||
// Move to intermission before Step 3 | ||
setHammerCount(hammerCount - 1); | ||
setScore((score ?? 0) + 25); | ||
setIsIntermission(true); // Show congratulations message | ||
} | ||
} else if (bonus === BonusType.Wave && tutorialStep === 3) { | ||
const bonusApplied = await applybonus({ | ||
bonus: BonusType.Wave, | ||
row_index: block.y, | ||
block_index: block.x, | ||
}); | ||
|
||
if (bonusApplied) { | ||
// Move to intermission before Step 4 | ||
setWaveCount(waveCount - 1); | ||
setScore((score ?? 0) + 400); | ||
setIsIntermission(true); | ||
} | ||
} else if (bonus === BonusType.Totem && tutorialStep === 4) { | ||
const bonusApplied = await applybonus({ | ||
bonus: BonusType.Totem, | ||
row_index: block.y, | ||
block_index: block.x, | ||
}); | ||
|
||
if (bonusApplied) { | ||
// Move to intermission before Step 4 | ||
setTotemCount(totemCount - 1); | ||
setScore((score ?? 0) + 250); | ||
setIsIntermission(true); | ||
} | ||
} | ||
|
||
// For tutorial step 1, track block movement | ||
if (tutorialStep === 1) { | ||
const isTargetBlock = block.y === 8 && block.x === 2; | ||
const finalPosition = Math.round(block.x); | ||
|
||
if (isTargetBlock && finalPosition === 2) { | ||
// Block moved to correct position | ||
setScore((score ?? 0) + 100); | ||
setIsIntermission(true); // Show congratulations message | ||
} | ||
} | ||
}, | ||
[bonus, tutorialStep], | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing dependencies to useCallback.
The selectBlock
callback uses several state variables (hammerCount
, waveCount
, totemCount
, score
) but they're not included in the dependencies array. This could lead to stale closures.
}, [
bonus,
tutorialStep,
+ hammerCount,
+ waveCount,
+ totemCount,
+ score,
]);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const selectBlock = useCallback( | |
async (block: Block) => { | |
if (bonus === BonusType.Hammer && tutorialStep === 2) { | |
const bonusApplied = await applybonus({ | |
bonus: BonusType.Hammer, | |
row_index: block.y, | |
block_index: block.x, | |
}); | |
if (bonusApplied) { | |
// Move to intermission before Step 3 | |
setHammerCount(hammerCount - 1); | |
setScore((score ?? 0) + 25); | |
setIsIntermission(true); // Show congratulations message | |
} | |
} else if (bonus === BonusType.Wave && tutorialStep === 3) { | |
const bonusApplied = await applybonus({ | |
bonus: BonusType.Wave, | |
row_index: block.y, | |
block_index: block.x, | |
}); | |
if (bonusApplied) { | |
// Move to intermission before Step 4 | |
setWaveCount(waveCount - 1); | |
setScore((score ?? 0) + 400); | |
setIsIntermission(true); | |
} | |
} else if (bonus === BonusType.Totem && tutorialStep === 4) { | |
const bonusApplied = await applybonus({ | |
bonus: BonusType.Totem, | |
row_index: block.y, | |
block_index: block.x, | |
}); | |
if (bonusApplied) { | |
// Move to intermission before Step 4 | |
setTotemCount(totemCount - 1); | |
setScore((score ?? 0) + 250); | |
setIsIntermission(true); | |
} | |
} | |
// For tutorial step 1, track block movement | |
if (tutorialStep === 1) { | |
const isTargetBlock = block.y === 8 && block.x === 2; | |
const finalPosition = Math.round(block.x); | |
if (isTargetBlock && finalPosition === 2) { | |
// Block moved to correct position | |
setScore((score ?? 0) + 100); | |
setIsIntermission(true); // Show congratulations message | |
} | |
} | |
}, | |
[bonus, tutorialStep], | |
); | |
const selectBlock = useCallback( | |
async (block: Block) => { | |
if (bonus === BonusType.Hammer && tutorialStep === 2) { | |
const bonusApplied = await applybonus({ | |
bonus: BonusType.Hammer, | |
row_index: block.y, | |
block_index: block.x, | |
}); | |
if (bonusApplied) { | |
// Move to intermission before Step 3 | |
setHammerCount(hammerCount - 1); | |
setScore((score ?? 0) + 25); | |
setIsIntermission(true); // Show congratulations message | |
} | |
} else if (bonus === BonusType.Wave && tutorialStep === 3) { | |
const bonusApplied = await applybonus({ | |
bonus: BonusType.Wave, | |
row_index: block.y, | |
block_index: block.x, | |
}); | |
if (bonusApplied) { | |
// Move to intermission before Step 4 | |
setWaveCount(waveCount - 1); | |
setScore((score ?? 0) + 400); | |
setIsIntermission(true); | |
} | |
} else if (bonus === BonusType.Totem && tutorialStep === 4) { | |
const bonusApplied = await applybonus({ | |
bonus: BonusType.Totem, | |
row_index: block.y, | |
block_index: block.x, | |
}); | |
if (bonusApplied) { | |
// Move to intermission before Step 4 | |
setTotemCount(totemCount - 1); | |
setScore((score ?? 0) + 250); | |
setIsIntermission(true); | |
} | |
} | |
// For tutorial step 1, track block movement | |
if (tutorialStep === 1) { | |
const isTargetBlock = block.y === 8 && block.x === 2; | |
const finalPosition = Math.round(block.x); | |
if (isTargetBlock && finalPosition === 2) { | |
// Block moved to correct position | |
setScore((score ?? 0) + 100); | |
setIsIntermission(true); // Show congratulations message | |
} | |
} | |
}, | |
[ | |
bonus, | |
tutorialStep, | |
hammerCount, | |
waveCount, | |
totemCount, | |
score, | |
], | |
); |
const handleStartTutorial = useCallback(() => { | ||
setTutorialState({ | ||
isActive: true, | ||
showGrid: true, | ||
showText: false, | ||
}); | ||
startTutorial(); | ||
}, [startTutorial]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant state initialization.
The handleStartTutorial
function redundantly sets state and then calls startTutorial
which sets state again. This could lead to unnecessary re-renders.
Simplify the implementation:
-const handleStartTutorial = useCallback(() => {
- setTutorialState({
- isActive: true,
- showGrid: true,
- showText: false,
- });
- startTutorial();
-}, [startTutorial]);
+const handleStartTutorial = useCallback(() => {
+ startTutorial();
+}, [startTutorial]);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleStartTutorial = useCallback(() => { | |
setTutorialState({ | |
isActive: true, | |
showGrid: true, | |
showText: false, | |
}); | |
startTutorial(); | |
}, [startTutorial]); | |
const handleStartTutorial = useCallback(() => { | |
startTutorial(); | |
}, [startTutorial]); |
const startTutorial = useCallback(() => { | ||
try { | ||
setTutorialState((prev) => ({ | ||
...prev, | ||
isActive: true, | ||
})); | ||
} catch (error) { | ||
console.error("Failed to start tutorial:", error); | ||
handleTutorialCleanup(); | ||
} | ||
}, [handleTutorialCleanup]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix redundant tutorial initialization and enhance error handling.
The startTutorial
function is called redundantly after state is already set in handleStartTutorial
. Additionally, error handling should provide user feedback.
Suggested improvements:
const startTutorial = useCallback(() => {
try {
setTutorialState((prev) => ({
...prev,
isActive: true,
+ currentStep: 0,
+ showGrid: true,
+ showText: false
}));
} catch (error) {
console.error("Failed to start tutorial:", error);
+ toast({
+ title: "Tutorial Error",
+ description: "Failed to start tutorial. Please try again.",
+ status: "error"
+ });
handleTutorialCleanup();
}
}, [handleTutorialCleanup]);
Committable suggestion skipped: line range outside the PR's diff.
const endTutorial = useCallback(() => { | ||
setTutorialState((prev) => ({ | ||
...prev, | ||
isActive: false, | ||
})); | ||
}, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add progress persistence to tutorial end handler.
The endTutorial
function should persist user progress before ending the tutorial.
Consider enhancing the end handler:
const endTutorial = useCallback(() => {
+ // Save progress before ending
+ if (tutorialState.currentStep > 0) {
+ localStorage.setItem('tutorialProgress', JSON.stringify({
+ lastStep: tutorialState.currentStep,
+ completedSteps: Array.from(tutorialState.completedSteps)
+ }));
+ }
setTutorialState((prev) => ({
...prev,
isActive: false,
}));
}, []);
Committable suggestion skipped: line range outside the PR's diff.
client/src/ui/screens/Home.tsx
Outdated
{/* {tutorialState.isActive && ( | ||
<Tutorial | ||
showGrid={tutorialState.showGrid} | ||
showTutorialText={tutorialState.showText} | ||
tutorial={tutorialState.isActive} | ||
endTutorial={endTutorial} | ||
/> | ||
)} */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uncomment and enhance Tutorial component integration.
The Tutorial component is currently commented out and lacks necessary props for step management as outlined in the PR objectives.
Suggested implementation:
-{/* {tutorialState.isActive && (
- <Tutorial
- showGrid={tutorialState.showGrid}
- showTutorialText={tutorialState.showText}
- tutorial={tutorialState.isActive}
- endTutorial={endTutorial}
- />
-)} */}
+{tutorialState.isActive && (
+ <Tutorial
+ showGrid={tutorialState.showGrid}
+ showTutorialText={tutorialState.showText}
+ tutorial={tutorialState.isActive}
+ currentStep={tutorialState.currentStep}
+ totalSteps={tutorialState.totalSteps}
+ completedSteps={tutorialState.completedSteps}
+ currentBonus={tutorialState.currentBonus}
+ onStepComplete={(step) => {
+ setTutorialState(prev => ({
+ ...prev,
+ completedSteps: new Set([...prev.completedSteps, step]),
+ currentStep: step + 1
+ }));
+ }}
+ endTutorial={endTutorial}
+ />
+)}
Committable suggestion skipped: line range outside the PR's diff.
// Tutorial state and handlers | ||
const [tutorialState, setTutorialState] = useState({ | ||
isActive: false, | ||
showGrid: false, | ||
showText: true, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance tutorial state management to support progression tracking.
The current tutorial state structure is too basic to support the required tutorial flow control and progression tracking outlined in the PR objectives.
Consider extending the state structure:
const [tutorialState, setTutorialState] = useState({
isActive: false,
showGrid: false,
showText: true,
+ currentStep: 0,
+ totalSteps: 3, // Number of tutorial steps
+ completedSteps: new Set<number>(),
+ currentBonus: null as 'hammer' | 'wave' | 'tiki' | null,
+ progression: {
+ basicMoves: false,
+ bonusUsage: false,
+ completed: false
+ }
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Tutorial state and handlers | |
const [tutorialState, setTutorialState] = useState({ | |
isActive: false, | |
showGrid: false, | |
showText: true, | |
}); | |
// Tutorial state and handlers | |
const [tutorialState, setTutorialState] = useState({ | |
isActive: false, | |
showGrid: false, | |
showText: true, | |
currentStep: 0, | |
totalSteps: 3, // Number of tutorial steps | |
completedSteps: new Set<number>(), | |
currentBonus: null as 'hammer' | 'wave' | 'tiki' | null, | |
progression: { | |
basicMoves: false, | |
bonusUsage: false, | |
completed: false | |
} | |
}); |
const endDrag = () => { | ||
if (!dragging) return; | ||
|
||
setBlocks((prevBlocks) => { | ||
const updatedBlocks = prevBlocks.map((b) => { | ||
if (b.id === dragging.id) { | ||
const finalX = Math.round(b.x); | ||
// if (Math.trunc(finalX) !== Math.trunc(initialX)) | ||
// setIsTxProcessing(true); | ||
setPendingMove({ | ||
block: b, | ||
rowIndex: b.y, | ||
startX: initialX, | ||
finalX, | ||
}); | ||
return { ...b, x: finalX }; | ||
} | ||
return b; | ||
}); | ||
return updatedBlocks; | ||
}); | ||
|
||
setDragging(null); | ||
setIsMoving(true); | ||
setGameState(GameState.GRAVITY); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the logic error in the endDrag
function.
The endDrag
function should set isTxProcessing
to true
when the final X position is different from the initial X position. However, the code that sets isTxProcessing
is currently commented out.
Apply this diff to fix the issue:
- // if (Math.trunc(finalX) !== Math.trunc(initialX))
- // setIsTxProcessing(true);
+ if (Math.trunc(finalX) !== Math.trunc(initialX)) {
+ setIsTxProcessing(true);
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const endDrag = () => { | |
if (!dragging) return; | |
setBlocks((prevBlocks) => { | |
const updatedBlocks = prevBlocks.map((b) => { | |
if (b.id === dragging.id) { | |
const finalX = Math.round(b.x); | |
// if (Math.trunc(finalX) !== Math.trunc(initialX)) | |
// setIsTxProcessing(true); | |
setPendingMove({ | |
block: b, | |
rowIndex: b.y, | |
startX: initialX, | |
finalX, | |
}); | |
return { ...b, x: finalX }; | |
} | |
return b; | |
}); | |
return updatedBlocks; | |
}); | |
setDragging(null); | |
setIsMoving(true); | |
setGameState(GameState.GRAVITY); | |
}; | |
const endDrag = () => { | |
if (!dragging) return; | |
setBlocks((prevBlocks) => { | |
const updatedBlocks = prevBlocks.map((b) => { | |
if (b.id === dragging.id) { | |
const finalX = Math.round(b.x); | |
if (Math.trunc(finalX) !== Math.trunc(initialX)) { | |
setIsTxProcessing(true); | |
} | |
setPendingMove({ | |
block: b, | |
rowIndex: b.y, | |
startX: initialX, | |
finalX, | |
}); | |
return { ...b, x: finalX }; | |
} | |
return b; | |
}); | |
return updatedBlocks; | |
}); | |
setDragging(null); | |
setIsMoving(true); | |
setGameState(GameState.GRAVITY); | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
client/src/dojo/game/models/game.ts
(2 hunks)client/src/dojo/game/types/mode.ts
(1 hunks)client/src/ui/screens/Home.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- client/src/dojo/game/models/game.ts
- client/src/dojo/game/types/mode.ts
🧰 Additional context used
🪛 Biome
client/src/ui/screens/Home.tsx
[error] 308-314: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (3)
client/src/ui/screens/Home.tsx (3)
325-354
: LGTM! Game over UI implementation looks good.
The game over UI effectively displays the game statistics and provides a clear end-game experience.
382-402
: LGTM! Game board integration is well implemented.
The GameBoard component properly handles game state and bonus counts, with appropriate checks for game over conditions.
176-183
:
Remove redundant state initialization.
The handleStartTutorial
function redundantly sets state and then calls startTutorial
which sets state again. This could lead to unnecessary re-renders.
Simplify the implementation:
-const handleStartTutorial = useCallback(() => {
- setTutorialState({
- isActive: true,
- showGrid: true,
- showText: false,
- });
- startTutorial();
-}, [startTutorial]);
+const handleStartTutorial = useCallback(() => {
+ setTutorialState(prev => ({
+ ...prev,
+ isActive: true,
+ showGrid: true,
+ showText: false
+ }));
+}, []);
Likely invalid or redundant comment.
{tutorialState.isActive ? ( | ||
<Tutorial | ||
showGrid={tutorialState.showGrid} | ||
showTutorialText={tutorialState.showText} | ||
tutorial={tutorialState.isActive} | ||
endTutorial={endTutorial} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance Tutorial component props for better flow control.
The current props structure doesn't provide enough control for managing complex tutorial flows as outlined in the PR objectives.
Enhance the Tutorial component interface:
<Tutorial
showGrid={tutorialState.showGrid}
showTutorialText={tutorialState.showText}
tutorial={tutorialState.isActive}
endTutorial={endTutorial}
+ currentStep={tutorialState.currentStep}
+ totalSteps={tutorialState.totalSteps}
+ onStepComplete={(step: number) => {
+ setTutorialState(prev => ({
+ ...prev,
+ completedSteps: new Set([...prev.completedSteps, step]),
+ currentStep: step + 1
+ }));
+ }}
+ currentBonus={tutorialState.currentBonus}
+ onBonusExplained={(bonus: string) => {
+ setTutorialState(prev => ({
+ ...prev,
+ progression: { ...prev.progression, bonusUsage: true }
+ }));
+ }}
/>
Committable suggestion skipped: line range outside the PR's diff.
const [tutorialState, setTutorialState] = useState({ | ||
isActive: false, | ||
showGrid: false, | ||
showText: true, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhance tutorial state to support progression tracking.
The current tutorial state structure is too basic to support the tutorial flow control requirements outlined in the PR objectives.
Consider extending the state:
const [tutorialState, setTutorialState] = useState({
isActive: false,
showGrid: false,
showText: true,
+ currentStep: 0,
+ totalSteps: TUTORIAL_STEPS.length,
+ completedSteps: new Set<number>(),
+ currentBonus: null as 'hammer' | 'wave' | 'tiki' | null,
+ progression: {
+ basicMoves: false,
+ bonusUsage: false,
+ completed: false
+ }
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const [tutorialState, setTutorialState] = useState({ | |
isActive: false, | |
showGrid: false, | |
showText: true, | |
}); | |
const [tutorialState, setTutorialState] = useState({ | |
isActive: false, | |
showGrid: false, | |
showText: true, | |
currentStep: 0, | |
totalSteps: TUTORIAL_STEPS.length, | |
completedSteps: new Set<number>(), | |
currentBonus: null as 'hammer' | 'wave' | 'tiki' | null, | |
progression: { | |
basicMoves: false, | |
bonusUsage: false, | |
completed: false | |
} | |
}); |
Hello ser @Cheelax I'm still waiting for your review on this |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
client/src/ui/components/DesktopHeader.tsx (1)
30-30
: Clean up consecutive empty lines.Multiple consecutive empty lines can be reduced to a single line for better code organization.
const [showTutorialText, setShowTutorialText] = useState(true); - - const handleTutorialOpen = () => { setIsTutorialOpen(true); }; - const handleTutorialClose = () => { setIsTutorialOpen(false); }; - const onClose = () => { setIsOpen(false); }; - - const navigate = useNavigate();Also applies to: 38-38, 47-48
client/src/ui/components/Grid.tsx (3)
Line range hint
160-180
: Improve bonus action error handlingThe bonus action handlers lack error handling for edge cases and don't provide user feedback.
if (bonus === BonusName.WAVE) { + try { setBlocks(removeBlocksSameRow(block, blocks)); + // Provide visual feedback + setAnimateText(ComboMessages.WAVE_BONUS); + } catch (error) { + console.error('Failed to apply WAVE bonus:', error); + // Reset state on error + setIsTxProcessing(false); + setGameState(GameState.WAITING); + } }
304-313
: Optimize fall distance calculationThe current implementation rechecks every position. We can optimize by tracking the last known collision.
const calculateFallDistance = (block: Block, blocks: Block[]) => { let maxFall = gridHeight - block.y - 1; + let lastCollisionY = -1; for (let y = block.y + 1; y < gridHeight; y++) { if (isCollision(block.x, y, block.width, blocks, block.id)) { maxFall = y - block.y - 1; + lastCollisionY = y; break; } } + // Cache last collision for future optimizations + block.lastCollisionY = lastCollisionY; return maxFall; };
488-530
: Enhance accessibility and performance of grid renderingThe grid rendering lacks accessibility attributes and could benefit from performance optimizations.
<motion.div animate={shouldBounce ? { scale: [1, 1.1, 1, 1.1, 1] } : {}} transition={{ duration: 0.2, ease: "easeInOut" }} + role="grid" + aria-label="Game Grid" > <div - className={`grid-background ${isTxProcessing ? " cursor-wait" : ""} `} + className={`grid-background${isTxProcessing ? " cursor-wait" : ""}`} + aria-busy={isTxProcessing} > <div className={`relative p-r-[1px] p-b-[1px] touch-action-none display-grid grid grid-cols-[repeat(${gridWidth},${gridSize}px)] grid-rows-[repeat(${gridHeight},${gridSize}px)] ${isPlayerInDanger ? " animated-box-player-danger" : ""}`} style={{ height: `${gridHeight * gridSize + borderSize}px`, width: `${gridWidth * gridSize + borderSize}px`, backgroundImage: "linear-gradient(#1E293B 2px, transparent 2px), linear-gradient(to right, #1E293B 2px, #10172A 2px)", backgroundSize: `${gridSize}px ${gridSize}px`, }} + role="presentation" >client/src/ui/screens/Home.tsx (1)
454-474
: Consider extracting GameBoard props preparation.The GameBoard component receives multiple props with similar conditional logic. Consider extracting this logic into a helper function.
+const prepareGameBoardProps = (game: Game, account: Account) => ({ + initialGrid: game.isOver() ? [] : game.blocks, + nextLine: game.isOver() ? [] : game.next_row, + score: game.isOver() ? 0 : game.score, + combo: game.isOver() ? 0 : game.combo, + maxCombo: game.isOver() ? 0 : game.max_combo, + hammerCount: game.isOver() ? 0 : game.hammer - game.hammer_used, + totemCount: game.isOver() ? 0 : game.totem - game.totem_used, + waveCount: game.isOver() ? 0 : game.wave - game.wave_used, + account, + game, +}); -<GameBoard - initialGrid={game.isOver() ? [] : game.blocks} - nextLine={game.isOver() ? [] : game.next_row} - score={game.isOver() ? 0 : game.score} - combo={game.isOver() ? 0 : game.combo} - maxCombo={game.isOver() ? 0 : game.max_combo} - hammerCount={game.isOver() ? 0 : game.hammer - game.hammer_used} - totemCount={game.isOver() ? 0 : game.totem - game.totem_used} - waveCount={game.isOver() ? 0 : game.wave - game.wave_used} - account={account} - game={game} -/> +<GameBoard {...prepareGameBoardProps(game, account)} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
client/src/hooks/useAccountCustom.tsx
(1 hunks)client/src/index.css
(1 hunks)client/src/ui/actions/Start.tsx
(1 hunks)client/src/ui/components/DesktopHeader.tsx
(2 hunks)client/src/ui/components/Grid.tsx
(14 hunks)client/src/ui/components/TreasureChest.tsx
(1 hunks)client/src/ui/screens/Home.tsx
(6 hunks)client/src/ui/screens/Loading.tsx
(1 hunks)client/src/utils/gridUtils.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- client/src/hooks/useAccountCustom.tsx
- client/src/index.css
- client/src/ui/actions/Start.tsx
- client/src/ui/components/TreasureChest.tsx
- client/src/ui/screens/Loading.tsx
- client/src/utils/gridUtils.ts
🧰 Additional context used
🪛 Biome
client/src/ui/screens/Home.tsx
[error] 394-394: Unexpected token. Did you mean {'}'}
or }
?
(parse)
[error] 520-521: Unexpected token. Did you mean {'}'}
or }
?
(parse)
[error] 521-521: expected <
but instead the file ends
the file ends here
(parse)
🔇 Additional comments (8)
client/src/ui/components/DesktopHeader.tsx (4)
13-13
: LGTM! Clean interface definition for tutorial functionality.
The new interface and import align well with the tutorial mode implementation objectives.
Also applies to: 18-20
21-21
: LGTM! Clean TypeScript component definition.
The component signature properly implements the DesktopHeaderProps interface.
31-37
: LGTM! Clean and focused handler implementations.
The tutorial handlers are well-structured and maintain clear separation of concerns.
Also applies to: 43-46
68-77
: LGTM! Clean UI integration of tutorial components.
The tutorial button and modal are well-integrated into the header layout. Good use of stopPropagation to prevent click event bubbling.
client/src/ui/screens/Home.tsx (4)
304-309
: 🛠️ Refactor suggestion
Enhance Tutorial component props for better flow control.
The current props structure doesn't provide enough control for managing complex tutorial flows as outlined in the PR objectives.
<Tutorial
showGrid={tutorialState.showGrid}
showTutorialText={tutorialState.showText}
tutorial={tutorialState.isActive}
endTutorial={endTutorial}
+ currentStep={tutorialState.currentStep}
+ totalSteps={tutorialState.totalSteps}
+ onStepComplete={(step: number) => {
+ setTutorialState(prev => ({
+ ...prev,
+ completedSteps: new Set([...prev.completedSteps, step]),
+ currentStep: step + 1
+ }));
+ }}
/>
Likely invalid or redundant comment.
166-176
: 🛠️ Refactor suggestion
Enhance error handling in the tutorial initialization.
The current error handling only logs to console, which doesn't provide feedback to users when tutorial initialization fails.
const startTutorial = useCallback(() => {
try {
setTutorialState(prev => ({
...prev,
isActive: true,
}));
} catch (error) {
console.error('Failed to start tutorial:', error);
+ toast({
+ title: "Tutorial Error",
+ description: "Failed to start tutorial. Please try again.",
+ status: "error"
+ });
handleTutorialCleanup();
}
}, [handleTutorialCleanup]);
Likely invalid or redundant comment.
178-185
:
Remove redundant state initialization.
The handleStartTutorial
function redundantly sets state and then calls startTutorial
which sets state again. This could lead to unnecessary re-renders.
-const handleStartTutorial = useCallback(() => {
- setTutorialState({
- isActive: true,
- showGrid: true,
- showText: false,
- });
- startTutorial();
-}, [startTutorial]);
+const handleStartTutorial = useCallback(() => {
+ setTutorialState(prev => ({
+ ...prev,
+ isActive: true,
+ showGrid: true,
+ showText: false
+ }));
+}, []);
Likely invalid or redundant comment.
151-156
: 🛠️ Refactor suggestion
Enhance tutorial state management to support progression tracking.
The current tutorial state structure is too basic to support the tutorial flow control requirements outlined in the PR objectives.
const [tutorialState, setTutorialState] = useState({
isActive: false,
showGrid: false,
showText: true,
+ currentStep: 0,
+ totalSteps: TUTORIAL_STEPS.length,
+ completedSteps: new Set<number>(),
+ progression: {
+ basicMoves: false,
+ bonusUsage: false,
+ completed: false
+ }
});
Likely invalid or redundant comment.
if (Math.trunc(finalX) !== Math.trunc(initialX)) | ||
setIsTxProcessing(true); | ||
setPendingMove({ | ||
block: b, | ||
rowIndex: b.y, | ||
startX: initialX, | ||
finalX, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential race condition in transaction processing
Setting isTxProcessing
immediately after checking finalX
could lead to a race condition if multiple moves are queued.
- if (Math.trunc(finalX) !== Math.trunc(initialX))
- setIsTxProcessing(true);
+ if (Math.trunc(finalX) !== Math.trunc(initialX)) {
+ // Ensure we're not already processing a transaction
+ if (!isTxProcessing) {
+ setIsTxProcessing(true);
+ } else {
+ return; // Skip if already processing
+ }
+ }
Committable suggestion skipped: line range outside the PR's diff.
const handleGravityState = (gravityState: GameState, newState: GameState) => { | ||
if ( | ||
gameState === gravityState && | ||
!isMoving && | ||
transitioningBlocks.length === 0 | ||
) { | ||
setGameState(newState); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve state transition handling
The handleGravityState
function could benefit from TypeScript type safety and state transition validation.
+ type GravityStateTransition = {
+ from: GameState;
+ to: GameState;
+ };
- const handleGravityState = (gravityState: GameState, newState: GameState) => {
+ const handleGravityState = ({ from, to }: GravityStateTransition) => {
if (
- gameState === gravityState &&
+ gameState === from &&
!isMoving &&
transitioningBlocks.length === 0
) {
- setGameState(newState);
+ setGameState(to);
}
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleGravityState = (gravityState: GameState, newState: GameState) => { | |
if ( | |
gameState === gravityState && | |
!isMoving && | |
transitioningBlocks.length === 0 | |
) { | |
setGameState(newState); | |
} | |
}; | |
type GravityStateTransition = { | |
from: GameState; | |
to: GameState; | |
}; | |
const handleGravityState = ({ from, to }: GravityStateTransition) => { | |
if ( | |
gameState === from && | |
!isMoving && | |
transitioningBlocks.length === 0 | |
) { | |
setGameState(to); | |
} | |
}; |
if (startColIndex === finalColIndex || isMoving) return; | ||
if (!account) return; | ||
|
||
isProcessingRef.current = true; | ||
setIsTxProcessing(true); | ||
playSwipe(); | ||
try { | ||
console.log( | ||
"Move TX (row, start col, end col)", | ||
gridHeight - 1 - rowIndex, | ||
startColIndex, | ||
finalColIndex, | ||
); | ||
await move({ | ||
account: account as Account, | ||
row_index: gridHeight - 1 - rowIndex, | ||
start_index: Math.trunc(startColIndex), | ||
final_index: Math.trunc(finalColIndex), | ||
}); | ||
console.log( | ||
`Mouvement effectué : Ligne ${rowIndex}, de ${startColIndex} à ${finalColIndex}`, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve transaction error handling and logging
The transaction error handling could be more robust and provide better debugging information.
const handleMoveTX = useCallback(
async (rowIndex: number, startColIndex: number, finalColIndex: number) => {
if (startColIndex === finalColIndex || isMoving) return;
if (!account) return;
setIsTxProcessing(true);
try {
+ console.log(
+ `Initiating move: Row ${rowIndex}, from ${startColIndex} to ${finalColIndex}`
+ );
await move({
account: account as Account,
row_index: gridHeight - 1 - rowIndex,
start_index: Math.trunc(startColIndex),
final_index: Math.trunc(finalColIndex),
});
- console.log(
- `Mouvement effectué : Ligne ${rowIndex}, de ${startColIndex} à ${finalColIndex}`,
- );
+ console.log('Move completed successfully');
} catch (error) {
- console.error("Erreur lors de l'envoi de la transaction", error);
+ console.error('Transaction failed:', {
+ error,
+ rowIndex,
+ startColIndex,
+ finalColIndex,
+ gridHeight
+ });
+ // Notify user of failure
+ setAnimateText(ComboMessages.ERROR);
setMoveTxAwaitDone(true);
- isProcessingRef.current = false; // Reset the ref
} finally {
- isProcessingRef.current = false; // Reset the ref
setMoveTxAwaitDone(true);
}
},
[account, isMoving, gridHeight, move],
);
Committable suggestion skipped: line range outside the PR's diff.
import React, { useCallback, useEffect, useState } from "react"; | ||
import "../../grid.css"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Based on the gathered information, I can now generate the final response:
Migration from BonusType to BonusName is incomplete and inconsistent
The codebase shows an incomplete migration from BonusType
to BonusName
enum:
Grid.tsx
uses the newBonusName
enum (WAVE, TIKI, HAMMER, NONE)- Most other components still use the old
BonusType
enum from@/dojo/game/types/bonus
- Values don't match between enums:
BonusType.Totem
vsBonusName.TIKI
- String values vs numeric enum
This will cause type conflicts and runtime errors. The migration needs to be completed consistently across all components.
🔗 Analysis chain
Props interface update needs validation handling
The change from BonusType
to BonusName
enum requires validation to ensure compatibility with existing bonus implementations.
Also applies to: 20-29
Let me gather more information about the actual usage and implementation to verify the compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining BonusType references that might cause type conflicts
rg -l "BonusType"
# Verify BonusName enum values match expected bonus types
ast-grep --pattern 'enum BonusName {
$$$
}'
Length of output: 593
Script:
#!/bin/bash
# Check how BonusType is used in the codebase
rg "BonusType" -A 3 -B 3
# Check how BonusName is used to ensure proper migration
rg "BonusName" -A 3 -B 3
# Look for any type definitions related to bonuses
ast-grep --pattern 'type $_ = {
$$$
bonus
$$$
}'
Length of output: 32836
client/src/ui/screens/Home.tsx
Outdated
)} | ||
{!!game && isGameOn === "isOver" && !isTournamentsOpen && ( | ||
<> | ||
<div className="flex flex-col gap-4 mt-4 md:mt-0"> | ||
<div className="p-6 rounded-lg shadow-lg w-full h-full bg-gray-900 m-2"> | ||
<p className="text-4xl text-center mb-2">Game Over</p> | ||
|
||
<div className="flex gap-4 justify-center items-center"> | ||
<div className="grow text-4xl flex gap-2 justify-end"> | ||
{game.score} | ||
<FontAwesomeIcon | ||
icon={faStar} | ||
className="text-yellow-500" | ||
/> | ||
</div> | ||
<div className="grow text-4xl flex gap-2 justify-end"> | ||
{game.combo} | ||
<FontAwesomeIcon | ||
icon={faFire} | ||
className="text-slate-700" | ||
/> | ||
</div> | ||
<div className="grow text-4xl flex gap-2 justify-end"> | ||
{game.max_combo} | ||
<MaxComboIcon | ||
width={36} | ||
height={36} | ||
className="text-slate-700" | ||
/> | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
|
||
{!isTournamentsOpen && ( | ||
<Dialog> | ||
<DialogTrigger asChild> | ||
<Button className="text-md md:text-2xl mt-2 md:p-4 p-2 bg-primary text-secondary rounded-lg"> | ||
Give feedback and get a chance to win STRK | ||
</Button> | ||
</DialogTrigger> | ||
|
||
<DialogContent className="sm:max-w-[700px] w-[95%] h-[580px] flex flex-col mx-auto justify-start items-center bg-opacity-50 rounded-lg shadow-lg"> | ||
<DialogHeader className="flex items-center"> | ||
<DialogTitle>Feedback</DialogTitle> | ||
</DialogHeader> | ||
<div className="flex-grow overflow-auto px-2 w-full h-full"> | ||
<GoogleFormEmbed /> | ||
</div> | ||
</DialogContent> | ||
</Dialog> | ||
)} | ||
</> | ||
)} | ||
{!!game && isGameOn === "isOn" && ( | ||
<div className="relative w-full"> | ||
<div | ||
ref={gameGrid} | ||
className="flex flex-col items-center game-container" | ||
> | ||
<GameBoard | ||
// Check if game is over because otherwise we can display | ||
// previous game data on the board while the new game is starting | ||
// and torii indexing | ||
initialGrid={game.isOver() ? [] : game.blocks} | ||
nextLine={game.isOver() ? [] : game.next_row} | ||
score={game.isOver() ? 0 : game.score} | ||
combo={game.isOver() ? 0 : game.combo} | ||
maxCombo={game.isOver() ? 0 : game.max_combo} | ||
hammerCount={ | ||
game.isOver() ? 0 : game.hammer - game.hammer_used | ||
} | ||
totemCount={ | ||
game.isOver() ? 0 : game.totem - game.totem_used | ||
} | ||
waveCount={ | ||
game.isOver() ? 0 : game.wave - game.wave_used | ||
} | ||
account={account} | ||
game={game} | ||
/> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Refactor duplicate game over UI code.
There's significant code duplication in the game over UI rendering logic. Consider extracting this into a separate component.
+const GameOverUI = ({ game }: { game: Game }) => (
+ <div className="flex flex-col gap-4 mt-4 md:mt-0">
+ <div className="p-6 rounded-lg shadow-lg w-full h-full bg-gray-900 m-2">
+ <p className="text-4xl text-center mb-2">Game Over</p>
+ <div className="flex gap-4 justify-center items-center">
+ <div className="grow text-4xl flex gap-2 justify-end">
+ {game.score}
+ <FontAwesomeIcon icon={faStar} className="text-yellow-500" />
+ </div>
+ <div className="grow text-4xl flex gap-2 justify-end">
+ {game.combo}
+ <FontAwesomeIcon icon={faFire} className="text-slate-700" />
+ </div>
+ <div className="grow text-4xl flex gap-2 justify-end">
+ {game.max_combo}
+ <MaxComboIcon width={36} height={36} className="text-slate-700" />
+ </div>
+ </div>
+ </div>
+ </div>
+);
// Replace duplicate game over UI with the new component
{!!game && isGameOn === "isOver" && <GameOverUI game={game} />}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
)} | |
{!!game && isGameOn === "isOver" && !isTournamentsOpen && ( | |
<> | |
<div className="flex flex-col gap-4 mt-4 md:mt-0"> | |
<div className="p-6 rounded-lg shadow-lg w-full h-full bg-gray-900 m-2"> | |
<p className="text-4xl text-center mb-2">Game Over</p> | |
<div className="flex gap-4 justify-center items-center"> | |
<div className="grow text-4xl flex gap-2 justify-end"> | |
{game.score} | |
<FontAwesomeIcon | |
icon={faStar} | |
className="text-yellow-500" | |
/> | |
</div> | |
<div className="grow text-4xl flex gap-2 justify-end"> | |
{game.combo} | |
<FontAwesomeIcon | |
icon={faFire} | |
className="text-slate-700" | |
/> | |
</div> | |
<div className="grow text-4xl flex gap-2 justify-end"> | |
{game.max_combo} | |
<MaxComboIcon | |
width={36} | |
height={36} | |
className="text-slate-700" | |
/> | |
</div> | |
</div> | |
</div> | |
</div> | |
{!isTournamentsOpen && ( | |
<Dialog> | |
<DialogTrigger asChild> | |
<Button className="text-md md:text-2xl mt-2 md:p-4 p-2 bg-primary text-secondary rounded-lg"> | |
Give feedback and get a chance to win STRK | |
</Button> | |
</DialogTrigger> | |
<DialogContent className="sm:max-w-[700px] w-[95%] h-[580px] flex flex-col mx-auto justify-start items-center bg-opacity-50 rounded-lg shadow-lg"> | |
<DialogHeader className="flex items-center"> | |
<DialogTitle>Feedback</DialogTitle> | |
</DialogHeader> | |
<div className="flex-grow overflow-auto px-2 w-full h-full"> | |
<GoogleFormEmbed /> | |
</div> | |
</DialogContent> | |
</Dialog> | |
)} | |
</> | |
)} | |
{!!game && isGameOn === "isOn" && ( | |
<div className="relative w-full"> | |
<div | |
ref={gameGrid} | |
className="flex flex-col items-center game-container" | |
> | |
<GameBoard | |
// Check if game is over because otherwise we can display | |
// previous game data on the board while the new game is starting | |
// and torii indexing | |
initialGrid={game.isOver() ? [] : game.blocks} | |
nextLine={game.isOver() ? [] : game.next_row} | |
score={game.isOver() ? 0 : game.score} | |
combo={game.isOver() ? 0 : game.combo} | |
maxCombo={game.isOver() ? 0 : game.max_combo} | |
hammerCount={ | |
game.isOver() ? 0 : game.hammer - game.hammer_used | |
} | |
totemCount={ | |
game.isOver() ? 0 : game.totem - game.totem_used | |
} | |
waveCount={ | |
game.isOver() ? 0 : game.wave - game.wave_used | |
} | |
account={account} | |
game={game} | |
/> | |
</div> | |
)} | |
{!!game && isGameOn === "isOver" && !isTournamentsOpen && ( | |
<> | |
<GameOverUI game={game} /> | |
{!isTournamentsOpen && ( | |
<Dialog> | |
<DialogTrigger asChild> | |
<Button className="text-md md:text-2xl mt-2 md:p-4 p-2 bg-primary text-secondary rounded-lg"> | |
Give feedback and get a chance to win STRK | |
</Button> | |
</DialogTrigger> | |
<DialogContent className="sm:max-w-[700px] w-[95%] h-[580px] flex flex-col mx-auto justify-start items-center bg-opacity-50 rounded-lg shadow-lg"> | |
<DialogHeader className="flex items-center"> | |
<DialogTitle>Feedback</DialogTitle> | |
</DialogHeader> | |
<div className="flex-grow overflow-auto px-2 w-full h-full"> | |
<GoogleFormEmbed /> | |
</div> | |
</DialogContent> | |
</Dialog> | |
)} | |
</> | |
)} | |
{!!game && isGameOn === "isOn" && ( | |
<div className="relative w-full"> | |
<div | |
ref={gameGrid} | |
className="flex flex-col items-center game-container" | |
> | |
<GameBoard | |
// Check if game is over because otherwise we can display | |
// previous game data on the board while the new game is starting | |
// and torii indexing | |
initialGrid={game.isOver() ? [] : game.blocks} | |
nextLine={game.isOver() ? [] : game.next_row} | |
score={game.isOver() ? 0 : game.score} | |
combo={game.isOver() ? 0 : game.combo} | |
maxCombo={game.isOver() ? 0 : game.max_combo} | |
hammerCount={ | |
game.isOver() ? 0 : game.hammer - game.hammer_used | |
} | |
totemCount={ | |
game.isOver() ? 0 : game.totem - game.totem_used | |
} | |
waveCount={ | |
game.isOver() ? 0 : game.wave - game.wave_used | |
} | |
account={account} | |
game={game} | |
/> | |
</div> |
🧰 Tools
🪛 Biome
[error] 394-394: Unexpected token. Did you mean {'}'}
or }
?
(parse)
@@ -5,7 +5,7 @@ import { Account } from "starknet"; | |||
type AccountType = "burner" | "controller"; | |||
|
|||
// eslint-disable-next-line prefer-const | |||
export let ACCOUNT_CONNECTOR: AccountType = "controller"; | |||
export let ACCOUNT_CONNECTOR: AccountType = "burner"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will break the game, revert plz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
client/src/ui/components/TutorialBlock.tsx (4)
5-24
: Add comprehensive JSDoc comments to the interface.While the interface is well-structured, adding detailed JSDoc comments would improve maintainability and developer experience.
interface BlockProps { + /** The block object containing position and dimension information */ block: Block; + /** The size of each grid cell in pixels */ gridSize: number; + /** Indicates if a transaction is being processed */ isTxProcessing?: boolean; + /** Duration of block transitions in milliseconds */ transitionDuration?: number; + /** Current game state affecting block behavior */ state?: GameState; + /** Handler for mouse down events */ handleMouseDown?: ( e: React.MouseEvent<HTMLDivElement>, block: BlockProps["block"], ) => void; + /** Handler for touch start events */ handleTouchStart?: ( e: React.TouchEvent<HTMLDivElement>, block: BlockProps["block"], ) => void; + /** Callback fired when block transition starts */ onTransitionBlockStart?: () => void; + /** Callback fired when block transition ends */ onTransitionBlockEnd?: () => void; + /** Whether the block should be visually highlighted */ isHighlighted?: boolean; + /** Type of highlight effect to apply */ highlightType?: "block" | "row"; + /** Whether the block can be interacted with */ isClickable?: boolean; }
40-41
: Consider using an enum for transition status.Using string literals for transition status could lead to maintenance issues. Consider using an enum instead.
+enum TransitionStatus { + START = "Start", + END = "End" +} -const [transitionStatus, setTransition] = useState("End"); +const [transitionStatus, setTransition] = useState<TransitionStatus>(TransitionStatus.END);
65-73
: Extract CSS classes into constants.Consider extracting CSS classes into named constants for better maintainability.
+const CSS_CLASSES = { + ROW_HIGHLIGHT: "ring-2 ring-yellow-400 ring-opacity-50", + BLOCK_HIGHLIGHT: "ring-4 ring-yellow-400 animate-pulse", +} as const; const highlightClass = isHighlighted ? highlightType === "row" - ? "ring-2 ring-yellow-400 ring-opacity-50" - : "ring-4 ring-yellow-400 animate-pulse" + ? CSS_CLASSES.ROW_HIGHLIGHT + ? CSS_CLASSES.BLOCK_HIGHLIGHT : "";
75-106
: Consider extracting styles to CSS/styled-components.The inline styles make the component harder to maintain. Consider moving them to a separate CSS file or using styled-components.
Also, consider extracting the magic number for zIndex (999) into a constant.
+const STYLES = { + Z_INDEX_HIGHLIGHTED: 999, + BASE_POSITION: "absolute", +}; style={{ - zIndex: isHighlighted ? 999 : "auto", + zIndex: isHighlighted ? STYLES.Z_INDEX_HIGHLIGHTED : "auto", - position: "absolute", + position: STYLES.BASE_POSITION, // ... rest of the styles }}client/src/ui/screens/Home.tsx (2)
166-176
: Enhance error handling with user feedback.The current error handling only logs to console, which doesn't provide feedback to users when tutorial initialization fails.
const startTutorial = useCallback(() => { try { setTutorialState((prev) => ({ ...prev, isActive: true, + showGrid: true, + showText: false })); } catch (error) { console.error("Failed to start tutorial:", error); + toast({ + title: "Tutorial Error", + description: "Failed to start tutorial. Please try again.", + status: "error" + }); handleTutorialCleanup(); } }, [handleTutorialCleanup]);
332-362
: Extract game over UI into a separate component.The game over UI code is duplicated. Consider extracting it into a reusable component for better maintainability.
+const GameOverUI = ({ game }: { game: Game }) => ( + <div className="flex flex-col gap-4 mt-4 md:mt-0"> + <div className="p-6 rounded-lg shadow-lg w-full h-full bg-gray-900 m-2"> + <p className="text-4xl text-center mb-2">Game Over</p> + <div className="flex gap-4 justify-center items-center"> + <div className="grow text-4xl flex gap-2 justify-end"> + {game.score} + <FontAwesomeIcon icon={faStar} className="text-yellow-500" /> + </div> + <div className="grow text-4xl flex gap-2 justify-end"> + {game.combo} + <FontAwesomeIcon icon={faFire} className="text-slate-700" /> + </div> + <div className="grow text-4xl flex gap-2 justify-end"> + {game.max_combo} + <MaxComboIcon width={36} height={36} className="text-slate-700" /> + </div> + </div> + </div> + </div> +); -<div className="flex flex-col gap-4 mt-4 md:mt-0"> - {/* ... existing game over UI code ... */} -</div> +<GameOverUI game={game} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
client/src/ui/components/TutorialBlock.tsx
(1 hunks)client/src/ui/screens/Home.tsx
(4 hunks)
🧰 Additional context used
🪛 Biome
client/src/ui/screens/Home.tsx
[error] 315-321: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (4)
client/src/ui/components/TutorialBlock.tsx (1)
26-39
: LGTM! Component declaration follows React best practices.
The component is well-structured with appropriate default values and handlers.
client/src/ui/screens/Home.tsx (3)
178-185
:
Remove redundant state initialization.
The handleStartTutorial
function redundantly sets state and then calls startTutorial
which sets state again. This could lead to unnecessary re-renders.
const handleStartTutorial = useCallback(() => {
- setTutorialState({
- isActive: true,
- showGrid: true,
- showText: false,
- });
startTutorial();
}, [startTutorial]);
Likely invalid or redundant comment.
151-156
: 🛠️ Refactor suggestion
Enhance tutorial state to support step-by-step progression.
The current tutorial state structure is too basic to support the guided gameplay requirements outlined in the PR objectives. Consider extending it to track tutorial steps and bonus explanations.
const [tutorialState, setTutorialState] = useState({
isActive: false,
showGrid: false,
showText: true,
+ currentStep: 0,
+ totalSteps: TUTORIAL_STEPS.length,
+ completedSteps: new Set<number>(),
+ currentBonus: null as 'hammer' | 'wave' | 'tiki' | null,
+ progression: {
+ basicMoves: false,
+ bonusUsage: false
+ }
});
Likely invalid or redundant comment.
304-309
: 🛠️ Refactor suggestion
Enhance Tutorial component interface for better flow control.
The current props structure doesn't provide enough control for managing complex tutorial flows as outlined in the PR objectives.
<Tutorial
showGrid={tutorialState.showGrid}
showTutorialText={tutorialState.showText}
tutorial={tutorialState.isActive}
endTutorial={endTutorial}
+ currentStep={tutorialState.currentStep}
+ totalSteps={tutorialState.totalSteps}
+ onStepComplete={(step: number) => {
+ setTutorialState(prev => ({
+ ...prev,
+ completedSteps: new Set([...prev.completedSteps, step]),
+ currentStep: step + 1
+ }));
+ }}
+ currentBonus={tutorialState.currentBonus}
+ onBonusExplained={(bonus: string) => {
+ setTutorialState(prev => ({
+ ...prev,
+ progression: { ...prev.progression, bonusUsage: true }
+ }));
+ }}
/>
Likely invalid or redundant comment.
useEffect(() => { | ||
if (ref.current === null) return; | ||
|
||
const onTransitionStart = () => { | ||
console.log("Transition started for block", block); | ||
onTransitionBlockStart(); | ||
setTransition("Start"); | ||
}; | ||
|
||
ref.current.addEventListener("transitionstart", onTransitionStart); | ||
|
||
return () => { | ||
ref.current?.removeEventListener("transitionstart", onTransitionStart); | ||
}; | ||
}, [block, onTransitionBlockStart]); | ||
|
||
const handleTransitionEnd = () => { | ||
console.log("Transition ended for block", block); | ||
setTransition("End"); | ||
onTransitionBlockEnd(); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove console.log statements from production code.
Console.log statements should be removed or replaced with proper logging utility.
const onTransitionStart = () => {
- console.log("Transition started for block", block);
onTransitionBlockStart();
setTransition("Start");
};
const handleTransitionEnd = () => {
- console.log("Transition ended for block", block);
setTransition("End");
onTransitionBlockEnd();
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
useEffect(() => { | |
if (ref.current === null) return; | |
const onTransitionStart = () => { | |
console.log("Transition started for block", block); | |
onTransitionBlockStart(); | |
setTransition("Start"); | |
}; | |
ref.current.addEventListener("transitionstart", onTransitionStart); | |
return () => { | |
ref.current?.removeEventListener("transitionstart", onTransitionStart); | |
}; | |
}, [block, onTransitionBlockStart]); | |
const handleTransitionEnd = () => { | |
console.log("Transition ended for block", block); | |
setTransition("End"); | |
onTransitionBlockEnd(); | |
}; | |
useEffect(() => { | |
if (ref.current === null) return; | |
const onTransitionStart = () => { | |
onTransitionBlockStart(); | |
setTransition("Start"); | |
}; | |
ref.current.addEventListener("transitionstart", onTransitionStart); | |
return () => { | |
ref.current?.removeEventListener("transitionstart", onTransitionStart); | |
}; | |
}, [block, onTransitionBlockStart]); | |
const handleTransitionEnd = () => { | |
setTransition("End"); | |
onTransitionBlockEnd(); | |
}; |
fix #22
Summary by CodeRabbit
Release Notes
New Features
BonusName
enum for better handling of bonus types.TutorialModal
component for an interactive tutorial experience.Tutorial
component that guides users through gameplay with interactive elements.TutorialGrid
component for managing grid-based interactions during the tutorial.BlockContainer
component for rendering block elements with transition effects.Improvements
Game
class for better handling of game state and block data.Start
component to include potential winnings and remaining time props.DesktopHeader
with tutorial functionality and state management.Grid
component by simplifying state management and drag event handling.Header
component to support tutorial initiation..highlight
CSS class for improved visual emphasis on UI elements.Bug Fixes
Chores